From 47ba4747b3f7864dd7fc6d50190f1f6824452499 Mon Sep 17 00:00:00 2001 From: KoloMl Date: Mon, 3 Feb 2025 23:20:20 +0400 Subject: [PATCH 1/3] Bail out early if submit button isn't present --- assets/js/upload.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/assets/js/upload.js b/assets/js/upload.js index 0f931037..de2196ad 100644 --- a/assets/js/upload.js +++ b/assets/js/upload.js @@ -231,11 +231,14 @@ function setupImageUpload() { function disableUploadButton() { const submitButton = $('.button.input--separate-top'); - if (submitButton !== null) { - submitButton.disabled = true; - submitButton.innerText = 'Please wait...'; + + if (!submitButton) { + return; } + submitButton.disabled = true; + submitButton.innerText = 'Please wait...'; + // delay is needed because Safari stops the submit if the button is immediately disabled requestAnimationFrame(() => submitButton.setAttribute('disabled', 'disabled')); } From 77352a3c773e675476cd84ee3ff53f15117fdad9 Mon Sep 17 00:00:00 2001 From: KoloMl Date: Mon, 3 Feb 2025 23:21:06 +0400 Subject: [PATCH 2/3] Detect when page is loaded from back-forward-cache and undo the disabling --- assets/js/upload.js | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/assets/js/upload.js b/assets/js/upload.js index de2196ad..7c129728 100644 --- a/assets/js/upload.js +++ b/assets/js/upload.js @@ -27,6 +27,27 @@ function elementForEmbeddedImage({ camo_url, type }) { return makeEl(tagName, { className: 'scraper-preview--image', src: objectUrl }); } +/** + * Execute the code when page was shown from back-forward cache. + * @param {() => void} callback + */ +function oncePersistedPageShown(callback) { + /** + * @param {PageTransitionEvent} event + */ + function onPageShown(event) { + if (!event.persisted) { + return; + } + + window.removeEventListener('pageshow', onPageShown); + + callback(); + } + + window.addEventListener('pageshow', onPageShown); +} + function setupImageUpload() { const imgPreviews = $('#js-image-upload-previews'); if (!imgPreviews) return; @@ -64,17 +85,21 @@ function setupImageUpload() { imgPreviews.appendChild(label); }); } + function showError() { clearEl(imgPreviews); showEl(scraperError); enableFetch(); } + function hideError() { hideEl(scraperError); } + function disableFetch() { fetchButton.setAttribute('disabled', ''); } + function enableFetch() { fetchButton.removeAttribute('disabled'); } @@ -236,11 +261,25 @@ function setupImageUpload() { return; } + const originalButtonText = submitButton.innerText; + submitButton.disabled = true; submitButton.innerText = 'Please wait...'; // delay is needed because Safari stops the submit if the button is immediately disabled requestAnimationFrame(() => submitButton.setAttribute('disabled', 'disabled')); + + // Rolling back the disabled state when user navigated back to the form. + oncePersistedPageShown(() => { + if (!submitButton.disabled) { + return; + } + + submitButton.disabled = false; + submitButton.innerText = originalButtonText; + + submitButton.removeAttribute('disabled'); + }); } function submitHandler(event) { From 3ecc59896e56ea7163871367aeb45ef519a6af21 Mon Sep 17 00:00:00 2001 From: KoloMl Date: Tue, 4 Feb 2025 01:12:06 +0400 Subject: [PATCH 3/3] Moved `oncePersistedPageShown` to events, added tests --- assets/js/upload.js | 22 +--------- assets/js/utils/__tests__/events.spec.ts | 55 +++++++++++++++++++++++- assets/js/utils/events.ts | 17 ++++++++ 3 files changed, 72 insertions(+), 22 deletions(-) diff --git a/assets/js/upload.js b/assets/js/upload.js index 7c129728..99145df4 100644 --- a/assets/js/upload.js +++ b/assets/js/upload.js @@ -6,6 +6,7 @@ import { assertNotNull } from './utils/assert'; import { fetchJson, handleError } from './utils/requests'; import { $, $$, clearEl, hideEl, makeEl, showEl } from './utils/dom'; import { addTag } from './tagsinput'; +import { oncePersistedPageShown } from './utils/events'; const MATROSKA_MAGIC = 0x1a45dfa3; @@ -27,27 +28,6 @@ function elementForEmbeddedImage({ camo_url, type }) { return makeEl(tagName, { className: 'scraper-preview--image', src: objectUrl }); } -/** - * Execute the code when page was shown from back-forward cache. - * @param {() => void} callback - */ -function oncePersistedPageShown(callback) { - /** - * @param {PageTransitionEvent} event - */ - function onPageShown(event) { - if (!event.persisted) { - return; - } - - window.removeEventListener('pageshow', onPageShown); - - callback(); - } - - window.addEventListener('pageshow', onPageShown); -} - function setupImageUpload() { const imgPreviews = $('#js-image-upload-previews'); if (!imgPreviews) return; diff --git a/assets/js/utils/__tests__/events.spec.ts b/assets/js/utils/__tests__/events.spec.ts index ab1dbd67..f53b22ce 100644 --- a/assets/js/utils/__tests__/events.spec.ts +++ b/assets/js/utils/__tests__/events.spec.ts @@ -1,4 +1,12 @@ -import { delegate, fire, mouseMoveThenOver, leftClick, on, PhilomenaAvailableEventsMap } from '../events'; +import { + delegate, + fire, + mouseMoveThenOver, + leftClick, + on, + PhilomenaAvailableEventsMap, + oncePersistedPageShown, +} from '../events'; import { getRandomArrayItem } from '../../../test/randomness'; import { fireEvent } from '@testing-library/dom'; @@ -129,6 +137,51 @@ describe('Event utils', () => { }); }); + describe('oncePersistedPageShown', () => { + it('should NOT fire on usual page show', () => { + const mockHandler = vi.fn(); + + oncePersistedPageShown(mockHandler); + + fireEvent.pageShow(window, { persisted: false }); + + expect(mockHandler).toHaveBeenCalledTimes(0); + }); + + it('should fire on persisted pageshow', () => { + const mockHandler = vi.fn(); + + oncePersistedPageShown(mockHandler); + + fireEvent.pageShow(window, { persisted: true }); + + expect(mockHandler).toHaveBeenCalledTimes(1); + }); + + it('should keep waiting until the first persistent page shown fired', () => { + const mockHandler = vi.fn(); + + oncePersistedPageShown(mockHandler); + + fireEvent.pageShow(window, { persisted: false }); + fireEvent.pageShow(window, { persisted: false }); + fireEvent.pageShow(window, { persisted: true }); + + expect(mockHandler).toHaveBeenCalledTimes(1); + }); + + it('should NOT fire more than once', () => { + const mockHandler = vi.fn(); + + oncePersistedPageShown(mockHandler); + + fireEvent.pageShow(window, { persisted: true }); + fireEvent.pageShow(window, { persisted: true }); + + expect(mockHandler).toHaveBeenCalledTimes(1); + }); + }); + describe('delegate', () => { it('should call the native addEventListener method on the element', () => { const mockElement = document.createElement('div'); diff --git a/assets/js/utils/events.ts b/assets/js/utils/events.ts index 458df039..f9d31c29 100644 --- a/assets/js/utils/events.ts +++ b/assets/js/utils/events.ts @@ -54,6 +54,23 @@ export function mouseMoveThenOver(element: El, func: (e: ); } +export function oncePersistedPageShown(func: (e: PageTransitionEvent) => void) { + const controller = new AbortController(); + + window.addEventListener( + 'pageshow', + (e: PageTransitionEvent) => { + if (!e.persisted) { + return; + } + + controller.abort(); + func(e); + }, + { signal: controller.signal }, + ); +} + export function delegate( node: PhilomenaEventElement, event: K,