From 55760ea57e2bf09338c6d031e4e31c0bc9cb7f43 Mon Sep 17 00:00:00 2001 From: wrenny-ko Date: Tue, 27 Aug 2024 14:58:27 -0400 Subject: [PATCH 01/10] client-side tag input validation on image upload submit, preserving the image in the form --- assets/js/upload.js | 116 +++++++++++++++++- .../templates/image/new.html.slime | 2 +- 2 files changed, 116 insertions(+), 2 deletions(-) diff --git a/assets/js/upload.js b/assets/js/upload.js index 16d33959..81d320e5 100644 --- a/assets/js/upload.js +++ b/assets/js/upload.js @@ -171,9 +171,123 @@ function setupImageUpload() { window.removeEventListener('beforeunload', beforeUnload); } + function createTagError(message) { + const buttonAfter = $('#tagsinput-save'); + const errorElement = makeEl('span', { className: 'help-block tag-error'}); + errorElement.innerText = message; + buttonAfter.parentElement.insertBefore(errorElement, buttonAfter); + } + + function clearTagErrors() { + const tagErrorElements = document.getElementsByClassName('tag-error'); + + // remove() causes the length to decrease + while(tagErrorElements.length > 0) { + tagErrorElements[0].remove(); + } + } + + // populate tag error helper bars as necessary + // return true if all checks pass + // return false if any check fails + function validateTags() { + const tags = $$('.tag'); + if (tags.length === 0) { + createTagError("Tag input must contain at least 3 tags") + return false; + } + + let tags_arr = []; + for (const i in tags) { + let tag = tags[i].innerText; + tag = tag.substring(0, tag.length - 2); // remove " x" from the end + tags_arr.push(tag); + } + + let ratings_tags = ["safe", "suggestive", "questionable", "explicit", + "semi-grimdark", "grimdark", "grotesque"]; + + let errors = []; + + let hasRating = false; + let hasSafe = false; + let hasOtherRating = false; + tags_arr.forEach(tag => { + if (ratings_tags.includes(tag)) { + hasRating = true; + if (tag === "safe") { + hasSafe = true; + } else { + hasOtherRating = true; + } + } + }); + + if (!hasRating) { + errors.push("Tag input must contain at least one rating tag"); + } else if (hasSafe && hasOtherRating) { + errors.push("Tag input may not contain any other rating if safe") + } + + if (tags_arr.length < 3) { + errors.push("Tag input must contain at least 3 tags"); + } + + errors.forEach(msg => createTagError(msg)); + + return errors.length == 0; // true: valid if no errors + } + + function enableUploadButton() { + const submitButton = $('.input--separate-top'); + if (submitButton !== null) { + submitButton.disabled = false; + submitButton.innerText = 'Upload'; + } + } + + function disableUploadButton() { + const submitButton = $('.input--separate-top'); + if (submitButton !== null) { + 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')); + } + + function anchorToTop() { + let url = window.location.href; + url = url.split('#')[0]; //remove any existing hash anchor from url + url += '#'; //move view to top of page + window.location.href = url; + } + + function submitHandler(event) { + clearTagErrors(); // remove any existing tag error elements + + if (validateTags() === true) { + // tags valid; + unregisterBeforeUnload(); + + // allow form submission + disableUploadButton(); + return true; + } else { + //tags invalid + enableUploadButton(); // enable Upload button + anchorToTop(); // move view to top of page + + // prevent form submission + event.preventDefault(); + return false; + } + } + fileField.addEventListener('change', registerBeforeUnload); fetchButton.addEventListener('click', registerBeforeUnload); - form.addEventListener('submit', unregisterBeforeUnload); + form.addEventListener('submit', submitHandler); } export { setupImageUpload }; diff --git a/lib/philomena_web/templates/image/new.html.slime b/lib/philomena_web/templates/image/new.html.slime index dfb664d9..2a080eb1 100644 --- a/lib/philomena_web/templates/image/new.html.slime +++ b/lib/philomena_web/templates/image/new.html.slime @@ -88,4 +88,4 @@ = render PhilomenaWeb.CaptchaView, "_captcha.html", name: "image", conn: @conn .actions - = submit "Upload", class: "button input--separate-top", autocomplete: "off", data: [disable_with: "Please wait..."] + = submit "Upload", class: "button input--separate-top", autocomplete: "off" From a4cad4e534fd66527b1436ee203d0dd2e3774df0 Mon Sep 17 00:00:00 2001 From: wrenny-ko Date: Tue, 27 Aug 2024 15:41:45 -0400 Subject: [PATCH 02/10] linting fixes --- assets/js/upload.js | 49 ++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/assets/js/upload.js b/assets/js/upload.js index 81d320e5..c31995bb 100644 --- a/assets/js/upload.js +++ b/assets/js/upload.js @@ -173,7 +173,7 @@ function setupImageUpload() { function createTagError(message) { const buttonAfter = $('#tagsinput-save'); - const errorElement = makeEl('span', { className: 'help-block tag-error'}); + const errorElement = makeEl('span', { className: 'help-block tag-error' }); errorElement.innerText = message; buttonAfter.parentElement.insertBefore(errorElement, buttonAfter); } @@ -182,7 +182,7 @@ function setupImageUpload() { const tagErrorElements = document.getElementsByClassName('tag-error'); // remove() causes the length to decrease - while(tagErrorElements.length > 0) { + while (tagErrorElements.length > 0) { tagErrorElements[0].remove(); } } @@ -193,29 +193,28 @@ function setupImageUpload() { function validateTags() { const tags = $$('.tag'); if (tags.length === 0) { - createTagError("Tag input must contain at least 3 tags") + createTagError('Tag input must contain at least 3 tags'); return false; } - let tags_arr = []; + const tagsArr = []; for (const i in tags) { let tag = tags[i].innerText; tag = tag.substring(0, tag.length - 2); // remove " x" from the end - tags_arr.push(tag); + tagsArr.push(tag); } - let ratings_tags = ["safe", "suggestive", "questionable", "explicit", - "semi-grimdark", "grimdark", "grotesque"]; + const ratingsTags = ['safe', 'suggestive', 'questionable', 'explicit', 'semi-grimdark', 'grimdark', 'grotesque']; - let errors = []; + const errors = []; let hasRating = false; let hasSafe = false; let hasOtherRating = false; - tags_arr.forEach(tag => { - if (ratings_tags.includes(tag)) { + tagsArr.forEach(tag => { + if (ratingsTags.includes(tag)) { hasRating = true; - if (tag === "safe") { + if (tag === 'safe') { hasSafe = true; } else { hasOtherRating = true; @@ -224,18 +223,18 @@ function setupImageUpload() { }); if (!hasRating) { - errors.push("Tag input must contain at least one rating tag"); + errors.push('Tag input must contain at least one rating tag'); } else if (hasSafe && hasOtherRating) { - errors.push("Tag input may not contain any other rating if safe") + errors.push('Tag input may not contain any other rating if safe'); } - if (tags_arr.length < 3) { - errors.push("Tag input must contain at least 3 tags"); + if (tagsArr.length < 3) { + errors.push('Tag input must contain at least 3 tags'); } errors.forEach(msg => createTagError(msg)); - return errors.length == 0; // true: valid if no errors + return errors.length === 0; // true: valid if no errors } function enableUploadButton() { @@ -250,7 +249,7 @@ function setupImageUpload() { const submitButton = $('.input--separate-top'); if (submitButton !== null) { submitButton.disabled = true; - submitButton.innerText = "Please wait..."; + submitButton.innerText = 'Please wait...'; } // delay is needed because Safari stops the submit if the button is immediately disabled @@ -274,15 +273,15 @@ function setupImageUpload() { // allow form submission disableUploadButton(); return true; - } else { - //tags invalid - enableUploadButton(); // enable Upload button - anchorToTop(); // move view to top of page - - // prevent form submission - event.preventDefault(); - return false; } + + //tags invalid + enableUploadButton(); // enable Upload button + anchorToTop(); // move view to top of page + + // prevent form submission + event.preventDefault(); + return false; } fileField.addEventListener('change', registerBeforeUnload); From c361118472b86b852c06a0801d9d1b58a37f5004 Mon Sep 17 00:00:00 2001 From: wrenny-ko Date: Tue, 27 Aug 2024 17:26:59 -0400 Subject: [PATCH 03/10] linting, js test troubleshooting --- assets/js/__tests__/upload.spec.ts | 16 ++++++++++++++++ assets/js/upload.js | 7 ++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/assets/js/__tests__/upload.spec.ts b/assets/js/__tests__/upload.spec.ts index 06d14d64..4647fbb4 100644 --- a/assets/js/__tests__/upload.spec.ts +++ b/assets/js/__tests__/upload.spec.ts @@ -58,6 +58,8 @@ describe('Image upload form', () => { let scraperError: HTMLDivElement; let fetchButton: HTMLButtonElement; let tagsEl: HTMLTextAreaElement; + let tagsinputEl: HTMLDivElement; + let tagEl: HTMLSpanElement; let sourceEl: HTMLInputElement; let descrEl: HTMLTextAreaElement; @@ -77,6 +79,18 @@ describe('Image upload form', () => { +
+ + "safe x" + + + "pony x" + + + "tag3 x" + +
+
-
`, ); diff --git a/assets/js/upload.js b/assets/js/upload.js index c904379d..8153fad8 100644 --- a/assets/js/upload.js +++ b/assets/js/upload.js @@ -248,11 +248,15 @@ function setupImageUpload() { requestAnimationFrame(() => submitButton.setAttribute('disabled', 'disabled')); } - function anchorToTop() { - let url = window.location.href; - url = url.split('#')[0]; //remove any existing hash anchor from url - url += '#taginput-fancy-tag_input'; //move view to tags input - window.location.href = url; + function scrollToTags() { + const taginputEle = $('#taginput-fancy-tag_input'); + + if (!taginputEle) { + // default to scroll to top + window.scrollTo({ top: 0, left: 0 }); + } + + taginputEle.scrollIntoView(); } function submitHandler(event) { @@ -268,8 +272,8 @@ function setupImageUpload() { // Let the form submission complete } else { - // Scroll to the top of page to see validation errors - anchorToTop(); + // Scroll to view validation errors + scrollToTags(); // allow users to re-submit the form enableUploadButton(); From 204e48d05b241ff6cceedbff5cd9576d43385524 Mon Sep 17 00:00:00 2001 From: wrenny-ko Date: Tue, 27 Aug 2024 19:16:13 -0400 Subject: [PATCH 07/10] mocking out a test that actually compiles --- assets/js/__tests__/upload.spec.ts | 21 +++++++++++++++++++-- assets/js/upload.js | 2 +- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/assets/js/__tests__/upload.spec.ts b/assets/js/__tests__/upload.spec.ts index ec0b986a..233bcd83 100644 --- a/assets/js/__tests__/upload.spec.ts +++ b/assets/js/__tests__/upload.spec.ts @@ -87,11 +87,11 @@ describe('Image upload form', () => { -
+
- +
`, ); @@ -210,4 +210,21 @@ describe('Image upload form', () => { expect(scraperError.innerText).toEqual('Error 1 Error 2'); }); }); + + it('should prevent form submission if tag checks fail', async () => { + await new Promise(resolve => { + form.addEventListener('submit', event => { + event.preventDefault(); + resolve(); + }); + fireEvent.submit(form); + }); + + const succeededUnloadEvent = new Event('beforeunload', { cancelable: true }); + expect(fireEvent(window, succeededUnloadEvent)).toBe(true); + await waitFor(() => { + assertSubmitButtonIsEnabled(); + expect(form.querySelectorAll('.help-block')).toHaveLength(1); + }); + }); }); diff --git a/assets/js/upload.js b/assets/js/upload.js index 8153fad8..fe068899 100644 --- a/assets/js/upload.js +++ b/assets/js/upload.js @@ -238,7 +238,7 @@ function setupImageUpload() { } function disableUploadButton() { - const submitButton = $('.input--separate-top'); + const submitButton = $('.button.input--separate-top'); if (submitButton !== null) { submitButton.disabled = true; submitButton.innerText = 'Please wait...'; From b713524989276b134cd030d66454a840db0b5726 Mon Sep 17 00:00:00 2001 From: wrenny-ko Date: Tue, 27 Aug 2024 19:29:41 -0400 Subject: [PATCH 08/10] setting up test --- assets/js/__tests__/upload.spec.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/assets/js/__tests__/upload.spec.ts b/assets/js/__tests__/upload.spec.ts index 233bcd83..f58132f1 100644 --- a/assets/js/__tests__/upload.spec.ts +++ b/assets/js/__tests__/upload.spec.ts @@ -25,6 +25,8 @@ const errorResponse = { }; /* eslint-enable camelcase */ +const tagSets = ['safe', 'one, two, three', 'safe, expicit', 'safe, two, three']; + describe('Image upload form', () => { let mockPng: File; let mockWebm: File; @@ -87,7 +89,7 @@ describe('Image upload form', () => { -
+
@@ -212,6 +214,14 @@ describe('Image upload form', () => { }); it('should prevent form submission if tag checks fail', async () => { + tagSets.forEach(tags => { + taginputEl.value = tags; + //TODO fire submit event + // check whether the form fully submitted or was prevented by tag checks + // verify the number of error help blocks added + // check if the submit button is enabled/disabled + }); + await new Promise(resolve => { form.addEventListener('submit', event => { event.preventDefault(); From 4010a8a277b8a6523025c88b97f1331439491497 Mon Sep 17 00:00:00 2001 From: wrenny-ko Date: Tue, 27 Aug 2024 20:02:23 -0400 Subject: [PATCH 09/10] tests for client side tag validation --- assets/js/__tests__/upload.spec.ts | 58 ++++++++++++++++++------------ 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/assets/js/__tests__/upload.spec.ts b/assets/js/__tests__/upload.spec.ts index f58132f1..bd5d9c5e 100644 --- a/assets/js/__tests__/upload.spec.ts +++ b/assets/js/__tests__/upload.spec.ts @@ -25,7 +25,8 @@ const errorResponse = { }; /* eslint-enable camelcase */ -const tagSets = ['safe', 'one, two, three', 'safe, expicit', 'safe, two, three']; +const tagSets = ['', 'a tag', 'safe', 'one, two, three', 'safe, explicit', 'safe, explicit, three', 'safe, two, three']; +const tagErrorCounts = [1, 2, 1, 1, 2, 1, 0]; describe('Image upload form', () => { let mockPng: File; @@ -213,28 +214,41 @@ describe('Image upload form', () => { }); }); + async function submitForm(frm): Promise { + return new Promise(resolve => { + function onSubmit() { + frm.removeEventListener('submit', onSubmit); + resolve(true); + } + + frm.addEventListener('submit', onSubmit); + + if (!fireEvent.submit(frm)) { + frm.removeEventListener('submit', onSubmit); + resolve(false); + } + }); + } + it('should prevent form submission if tag checks fail', async () => { - tagSets.forEach(tags => { - taginputEl.value = tags; - //TODO fire submit event - // check whether the form fully submitted or was prevented by tag checks - // verify the number of error help blocks added - // check if the submit button is enabled/disabled - }); + for (let i = 0; i < tagSets.length; i += 1) { + taginputEl.value = tagSets[i]; - await new Promise(resolve => { - form.addEventListener('submit', event => { - event.preventDefault(); - resolve(); - }); - fireEvent.submit(form); - }); - - const succeededUnloadEvent = new Event('beforeunload', { cancelable: true }); - expect(fireEvent(window, succeededUnloadEvent)).toBe(true); - await waitFor(() => { - assertSubmitButtonIsEnabled(); - expect(form.querySelectorAll('.help-block')).toHaveLength(1); - }); + if (await submitForm(form)) { + // form submit succeeded + await waitFor(() => { + assertSubmitButtonIsDisabled(); + const succeededUnloadEvent = new Event('beforeunload', { cancelable: true }); + expect(fireEvent(window, succeededUnloadEvent)).toBe(true); + }); + } else { + // form submit prevented + frm = form; + await waitFor(() => { + assertSubmitButtonIsEnabled(); + expect(frm.querySelectorAll('.help-block')).toHaveLength(tagErrorCounts[i]); + }); + } + } }); }); From 343078678a1747d2a6d0d031395017bc169798f3 Mon Sep 17 00:00:00 2001 From: wrenny-ko Date: Thu, 29 Aug 2024 18:35:20 -0400 Subject: [PATCH 10/10] scroll to tag block, review suggestions, cleanup --- assets/js/upload.js | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/assets/js/upload.js b/assets/js/upload.js index fe068899..0f931037 100644 --- a/assets/js/upload.js +++ b/assets/js/upload.js @@ -2,6 +2,7 @@ * Fetch and display preview images for various image upload forms. */ +import { assertNotNull } from './utils/assert'; import { fetchJson, handleError } from './utils/requests'; import { $, $$, clearEl, hideEl, makeEl, showEl } from './utils/dom'; import { addTag } from './tagsinput'; @@ -173,9 +174,8 @@ function setupImageUpload() { function createTagError(message) { const buttonAfter = $('#tagsinput-save'); - const errorElement = makeEl('span', { className: 'help-block tag-error' }); + const errorElement = makeEl('span', { className: 'help-block tag-error', innerText: message }); - errorElement.innerText = message; buttonAfter.insertAdjacentElement('beforebegin', errorElement); } @@ -229,14 +229,6 @@ function setupImageUpload() { return errors.length === 0; // true: valid if no errors } - function enableUploadButton() { - const submitButton = $('.input--separate-top'); - if (submitButton !== null) { - submitButton.disabled = false; - submitButton.innerText = 'Upload'; - } - } - function disableUploadButton() { const submitButton = $('.button.input--separate-top'); if (submitButton !== null) { @@ -248,17 +240,6 @@ function setupImageUpload() { requestAnimationFrame(() => submitButton.setAttribute('disabled', 'disabled')); } - function scrollToTags() { - const taginputEle = $('#taginput-fancy-tag_input'); - - if (!taginputEle) { - // default to scroll to top - window.scrollTo({ top: 0, left: 0 }); - } - - taginputEle.scrollIntoView(); - } - function submitHandler(event) { // Remove any existing tag error elements clearTagErrors(); @@ -273,10 +254,7 @@ function setupImageUpload() { // Let the form submission complete } else { // Scroll to view validation errors - scrollToTags(); - - // allow users to re-submit the form - enableUploadButton(); + assertNotNull($('.fancy-tag-upload')).scrollIntoView(); // Prevent the form from being submitted event.preventDefault();