From 8cd43416be24806421d569a50216054173021abc Mon Sep 17 00:00:00 2001 From: Splines Date: Sun, 12 Nov 2023 16:19:37 +0100 Subject: [PATCH 01/20] Reduce complexity of small/large display checks Also removed deprecated device-width, see: https://stackoverflow.com/a/18500871/ --- .../javascripts/thyme/display_manager.js | 100 +++--------------- 1 file changed, 15 insertions(+), 85 deletions(-) diff --git a/app/assets/javascripts/thyme/display_manager.js b/app/assets/javascripts/thyme/display_manager.js index 7fac4e32f..a65985242 100644 --- a/app/assets/javascripts/thyme/display_manager.js +++ b/app/assets/javascripts/thyme/display_manager.js @@ -16,8 +16,6 @@ class DisplayManager { this.onEnlarge = onEnlarge; } - - // on small display, fall back to standard browser player adaptToSmallDisplay() { for (let e of this.elements) { @@ -38,93 +36,25 @@ class DisplayManager { // Check screen size and trigger the right method updateControlBarType() { - const dm = this; - - if (window.matchMedia("screen and (max-width: " + - thymeAttributes.hideControlBarThreshold.x + "px)").matches || - window.matchMedia("screen and (max-height: " + - thymeAttributes.hideControlBarThreshold.y + "px)").matches) { - dm.adaptToSmallDisplay(); - } + const manager = this; - if (window.matchMedia("screen and (max-device-width: " + - thymeAttributes.hideControlBarThreshold.x + "px)").matches || - window.matchMedia("screen and (max-device-height: " + - thymeAttributes.hideControlBarThreshold.y + "px)").matches) { - dm.adaptToSmallDisplay(); - } - - // mediaQuery listener for very small screens - const matchVerysmallX = window.matchMedia("screen and (max-width: " + - thymeAttributes.hideControlBarThreshold.x + "px)"); - matchVerysmallX.addListener(function (result) { - if (result.matches) { - dm.adaptToSmallDisplay(); - } - }); - const matchVerysmallY = window.matchMedia("screen and (max-height: " + - thymeAttributes.hideControlBarThreshold.y + "px)"); - matchVerysmallY.addListener(function (result) { - if (result.matches) { - dm.adaptToSmallDisplay(); - } - }); + const matchSmallMediaQuery = window.matchMedia(` + screen and ( + (max-width: ${thymeAttributes.hideControlBarThreshold.x}px) + or (max-height: ${thymeAttributes.hideControlBarThreshold.y}px) + ) + `); - const matchVerysmalldeviceX = window.matchMedia("screen and (max-device-width: " + - thymeAttributes.hideControlBarThreshold.x + "px)"); - matchVerysmalldeviceX.addListener(function (result) { - if (result.matches) { - dm.adaptToSmallDisplay(); + function handleSizeChange(event) { + if (event.matches) { + manager.adaptToSmallDisplay(); + } else { + manager.adaptToLargeDisplay(); } - }); - const matchVerysmalldeviceY = window.matchMedia("screen and (max-device-height: " + - thymeAttributes.hideControlBarThreshold.y + "px)"); - matchVerysmalldeviceY.addListener(function (result) { - if (result.matches) { - dm.adaptToSmallDisplay(); - } - }); - - // mediaQuery listener for normal screens - let matchNormalX = window.matchMedia("screen and (min-width: " + - (thymeAttributes.hideControlBarThreshold.x + 1) + "px)"); - matchNormalX.addListener(function (result) { - let matchNormalY; - matchNormalY = window.matchMedia("screen and (min-height: " + - (thymeAttributes.hideControlBarThreshold.y + 1) + "px)"); - if (result.matches && matchNormalY.matches) { - dm.adaptToLargeDisplay(); - } - }); - const matchNormalY = window.matchMedia("screen and (min-height: " + - (thymeAttributes.hideControlBarThreshold.y + 1) + "px)"); - matchNormalY.addListener(function (result) { - matchNormalX = window.matchMedia("screen and (min-width: " + - (thymeAttributes.hideControlBarThreshold.x + 1) + "px)"); - if (result.matches && matchNormalX.matches) { - dm.adaptToLargeDisplay(); - } - }); + } - let matchNormaldeviceX = window.matchMedia("screen and (min-device-width: " + - (thymeAttributes.hideControlBarThreshold.x + 1) + "px)"); - let matchNormaldeviceY; - matchNormaldeviceX.addListener(function (result) { - matchNormaldeviceY = window.matchMedia("screen and (min-device-height: " + - (thymeAttributes.hideControlBarThreshold.y + 1) + "px)"); - if (result.matches && matchNormalY.matches) { - dm.adaptToLargeDisplay(); - } - }); - matchNormaldeviceY = window.matchMedia("screen and (min-device-height: " + - (thymeAttributes.hideControlBarThreshold.y + 1) + "px)"); - matchNormaldeviceY.addListener(function (result) { - matchNormaldeviceX = window.matchMedia("screen and (min-device-width: " + - (thymeAttributes.hideControlBarThreshold.x + 1) + "px)"); - if (result.matches && matchNormalX.matches) { - dm.adaptToLargeDisplay(); - } - }); + matchSmallMediaQuery.addListener(handleSizeChange); + handleSizeChange(matchSmallMediaQuery); // initial call } }; From 958af8fdd9da3f8840ecdb8164e2a23d6a6ff85b Mon Sep 17 00:00:00 2001 From: Splines Date: Sun, 12 Nov 2023 16:24:47 +0100 Subject: [PATCH 02/20] Implement locking to prevent unnecessary DB calls The AnnotationManager should handle its internal state on its own. Therefore, updateMarkers() now checks if annotations are null and if they are, it calls updateAnnotations() accordingly, but only if no other method has already called updateMarkers recently, e.g. when multiple resize events are fired in a small period of time. Resource is freed in updateAnnotations. --- .../thyme/annotations/annotation_manager.js | 25 +++++++++++++++---- .../javascripts/thyme/thyme_feedback.js | 6 +---- app/assets/javascripts/thyme/thyme_player.js | 6 +---- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/app/assets/javascripts/thyme/annotations/annotation_manager.js b/app/assets/javascripts/thyme/annotations/annotation_manager.js index 19b43f0d8..3e5ab05d0 100644 --- a/app/assets/javascripts/thyme/annotations/annotation_manager.js +++ b/app/assets/javascripts/thyme/annotations/annotation_manager.js @@ -33,10 +33,9 @@ class AnnotationManager { this.onClick = onClick; this.onUpdate = onUpdate; this.isValid = isValid; + this.isDbCalledForFreshAnnotations = false; } - - /* Updates the markers on the timeline, i.e. the visual represention of the annotations. This method is e.g. used for rearranging the markers when the window is being resized. @@ -44,9 +43,19 @@ class AnnotationManager { in the database. */ updateMarkers() { + // In case the annotations have not been loaded yet, do nothing. + // This situation might occur during the initial page load. + if (!thymeAttributes.annotations) { + if (!this.isDbCalledForFreshAnnotations) { + this.updateAnnotations(); + } + return; + } + const annotationManager = this; $('#' + thymeAttributes.markerBarId).empty(); AnnotationManager.sortAnnotations(); + for (const a of thymeAttributes.annotations) { if (this.isValid(a)) { function onClick() { @@ -72,14 +81,16 @@ class AnnotationManager { successfully updated. */ updateAnnotations() { - const annotationManager = this; + this.isDbCalledForFreshAnnotations = true; // Lock resource + + const manager = this; $.ajax(Routes.update_annotations_path(), { type: 'GET', dataType: 'json', data: { mediumId: thymeAttributes.mediumId, }, - success: function(annotations) { + success: (annotations) => { // update the annotation field in thymeAttributes thymeAttributes.annotations = []; if (!annotations) { @@ -89,7 +100,11 @@ class AnnotationManager { thymeAttributes.annotations.push(new Annotation(a)); } // update visual representation on the seek bar - annotationManager.updateMarkers(); + manager.updateMarkers(); + }, + always: () => { + // Free resource + manager.isDbCalledForFreshAnnotations = false; } }); } diff --git a/app/assets/javascripts/thyme/thyme_feedback.js b/app/assets/javascripts/thyme/thyme_feedback.js index b7eaae1e9..3f4e5e326 100644 --- a/app/assets/javascripts/thyme/thyme_feedback.js +++ b/app/assets/javascripts/thyme/thyme_feedback.js @@ -106,11 +106,7 @@ $(document).on('turbolinks:load', function() { // resizes the thyme container to the window dimensions function resizeContainer() { resize.resizeContainer(thymeContainer, 1.22, 70); - if (!thymeAttributes.annotations) { - annotationManager.updateAnnotations(); - } else { - annotationManager.updateMarkers(); - } + annotationManager.updateMarkers(); }; window.onresize = resizeContainer; video.onloadedmetadata = resizeContainer; diff --git a/app/assets/javascripts/thyme/thyme_player.js b/app/assets/javascripts/thyme/thyme_player.js index 18e02ec27..07c3c2cb3 100644 --- a/app/assets/javascripts/thyme/thyme_player.js +++ b/app/assets/javascripts/thyme/thyme_player.js @@ -152,11 +152,7 @@ $(document).on('turbolinks:load', function() { function resizeContainer() { const factor = $('#caption').is(':hidden') && $('#annotation-caption').is(':hidden') ? 1 : 1 / 0.82; resize.resizeContainer(thymeContainer, factor, 0); - if (!thymeAttributes.annotations) { - annotationManager.updateAnnotations(); - } else { - annotationManager.updateMarkers(); - } + annotationManager.updateMarkers(); }; window.onresize = resizeContainer; video.onloadedmetadata = resizeContainer; From 8fcc2389dd4d3c946a328bfd2789b002285ce277 Mon Sep 17 00:00:00 2001 From: Splines Date: Sun, 12 Nov 2023 17:08:08 +0100 Subject: [PATCH 03/20] Rewrite handling of annotation update --- app/assets/javascripts/thyme/thyme_player.js | 9 --------- app/views/annotations/_annotation_modal.html.erb | 4 +--- app/views/annotations/update.js | 3 ++- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/thyme/thyme_player.js b/app/assets/javascripts/thyme/thyme_player.js index 7a80c6f26..f1a05f9cb 100644 --- a/app/assets/javascripts/thyme/thyme_player.js +++ b/app/assets/javascripts/thyme/thyme_player.js @@ -99,15 +99,6 @@ $(document).on('turbolinks:load', function() { onClick, onUpdate, isValid); thymeAttributes.annotationManager = annotationManager; - // Update annotations after submitting the annotations form - $('#annotation-update').on('DOMSubtreeModified', function() { - const updateDataset = document.querySelector('#annotation-update').dataset; - if (updateDataset.update === "updated") { - updateDataset.update = ""; - annotationManager.updateAnnotations(); - } - }); - // Update annotations after deleting an annotation $(document).on('click', '#delete-button', function() { const annotationId = Number(document.getElementById('annotation_id').textContent); diff --git a/app/views/annotations/_annotation_modal.html.erb b/app/views/annotations/_annotation_modal.html.erb index 4cd7e63e8..b44abe9c3 100644 --- a/app/views/annotations/_annotation_modal.html.erb +++ b/app/views/annotations/_annotation_modal.html.erb @@ -24,7 +24,5 @@ + - - \ No newline at end of file diff --git a/app/views/annotations/update.js b/app/views/annotations/update.js index 3df69e3ef..2cc509f01 100644 --- a/app/views/annotations/update.js +++ b/app/views/annotations/update.js @@ -1,3 +1,4 @@ $('#annotation-modal').modal("hide"); -document.querySelector('#annotation-update').dataset.update = "updated"; \ No newline at end of file +// Update annotations after submitting the annotations form +thymeAttributes.annotationManager.updateAnnotations(); From 741267f6e728d6eb450531ada30405db59d70e13 Mon Sep 17 00:00:00 2001 From: Splines Date: Sun, 12 Nov 2023 17:45:12 +0100 Subject: [PATCH 04/20] Use gender-neutral pronoun --- config/locales/en.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index ebf84ef03..cc1609f40 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -384,7 +384,7 @@ en: visible_for_teacher: 'Visible for teacher?' visible_for_teacher_helpdesk: > If this checkbox is selected your teacher can see this annotation - (without your name!) in his own thyme player. + (without your name!) in their own thyme player. post_as_comment: 'Post as comment?' post_as_comment_helpdesk: > If this checkbox is selected, this annotation will be published as From 0a426d1b17b9e6b833bc75befa1dfd3769664a6a Mon Sep 17 00:00:00 2001 From: Splines Date: Mon, 13 Nov 2023 19:20:38 +0100 Subject: [PATCH 05/20] Replace alert() by Bootstrap alert Used for "Post as comment" warnings. --- app/views/annotations/_form.html.erb | 3 ++ app/views/annotations/edit.js.erb | 65 +++++++++++++++------------- config/locales/de.yml | 4 +- config/locales/en.yml | 6 +-- 4 files changed, 43 insertions(+), 35 deletions(-) diff --git a/app/views/annotations/_form.html.erb b/app/views/annotations/_form.html.erb index e25155275..0716d9325 100644 --- a/app/views/annotations/_form.html.erb +++ b/app/views/annotations/_form.html.erb @@ -130,6 +130,9 @@ <%= helpdesk(t('admin.annotation.post_as_comment_helpdesk'), false) %> + +