From 80ba5694a75b70aa32fd59c72290ef901f6df1bc Mon Sep 17 00:00:00 2001 From: Rebecca Shoptaw Date: Mon, 29 Jan 2024 15:36:16 -0500 Subject: [PATCH 1/6] Remove input clearing from formdata function. --- openlibrary/plugins/openlibrary/js/jquery.repeat.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/openlibrary/plugins/openlibrary/js/jquery.repeat.js b/openlibrary/plugins/openlibrary/js/jquery.repeat.js index 6df3c15dbd2..9537511ec38 100644 --- a/openlibrary/plugins/openlibrary/js/jquery.repeat.js +++ b/openlibrary/plugins/openlibrary/js/jquery.repeat.js @@ -39,25 +39,19 @@ export default function($){ /** * Search elems.form for input fields and create an * object representing. - * This function has side effects and will reset any - * input[type=text] fields it has found in the process * @return {object} data mapping names to values */ function formdata() { var data = {}; $(':input', elems.form).each(function() { var $e = $(this), - type = $e.attr('type'), name = $e.attr('name'); data[name] = $e.val().trim(); - // reset the values we are copying across - if (type === 'text') { - $e.val(''); - } }); return data; } + /** * triggered when "add link" button is clicked on author edit field. * Creates a removable `repeat-item`. From d37dfef28aae7bef237818f2f8a831104f2065ec Mon Sep 17 00:00:00 2001 From: Rebecca Shoptaw Date: Mon, 29 Jan 2024 15:36:37 -0500 Subject: [PATCH 2/6] Add input clearing after passed error checks. --- openlibrary/plugins/openlibrary/js/edit.js | 26 ++++++++++++++-------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/openlibrary/plugins/openlibrary/js/edit.js b/openlibrary/plugins/openlibrary/js/edit.js index 5c216e84bc8..8b9cbf41d55 100644 --- a/openlibrary/plugins/openlibrary/js/edit.js +++ b/openlibrary/plugins/openlibrary/js/edit.js @@ -79,6 +79,7 @@ export function initRoleValidation() { return error('#role-errors', '#role-name', dataConfig['You need to give this ROLE a name.'].replace(/ROLE/, data.role)); } $('#role-errors').hide(); + $('#select-role, #role-name').val(''); return true; } }); @@ -97,7 +98,10 @@ export function isbnConfirmAdd(data) { const yesButtonSelector = '#yes-add-isbn' const noButtonSelector = '#do-not-add-isbn' - const onYes = () => {$('#id-errors').hide()}; + const onYes = () => { + $('#id-errors').hide(); + $('#select-id, #id-value').val(''); + }; const onNo = () => { $('#id-errors').hide(); isbnOverride.clear(); @@ -122,8 +126,8 @@ export function isbnConfirmAdd(data) { function validateIsbn10(data, dataConfig, label) { data.value = parseIsbn(data.value); - if (isFormatValidIsbn10(data.value) === false) { - return error('#id-errors', 'id-value', dataConfig['ID must be exactly 10 characters [0-9] or X.'].replace(/ID/, label)); + if (!isFormatValidIsbn10(data.value)) { + return error('#id-errors', '#id-value', dataConfig['ID must be exactly 10 characters [0-9] or X.'].replace(/ID/, label)); } // Here the ISBN has a valid format, but also has an invalid checksum. Give the user a chance to verify // the ISBN, as books sometimes issue with invalid ISBNs and we want to be able to add them. @@ -147,7 +151,7 @@ function validateIsbn13(data, dataConfig, label) { data.value = parseIsbn(data.value); if (isFormatValidIsbn13(data.value) === false) { - return error('#id-errors', 'id-value', dataConfig['ID must be exactly 13 digits [0-9]. For example: 978-1-56619-909-4'].replace(/ID/, label)); + return error('#id-errors', '#id-value', dataConfig['ID must be exactly 13 digits [0-9]. For example: 978-1-56619-909-4'].replace(/ID/, label)); } // Here the ISBN has a valid format, but also has an invalid checksum. Give the user a chance to verify // the ISBN, as books sometimes issue with invalid ISBNs and we want to be able to add them. @@ -171,7 +175,7 @@ function validateLccn(data, dataConfig, label) { data.value = parseLccn(data.value); if (isValidLccn(data.value) === false) { - return error('#id-errors', 'id-value', dataConfig['Invalid ID format'].replace(/ID/, label)); + return error('#id-errors', '#id-value', dataConfig['Invalid ID format'].replace(/ID/, label)); } return true; } @@ -187,14 +191,14 @@ export function validateIdentifiers(data) { const dataConfig = JSON.parse(document.querySelector('#identifiers').dataset.config); if (data.name === '' || data.name === '---') { - return error('#id-errors', 'select-id', dataConfig['Please select an identifier.']) + return error('#id-errors', '#select-id', dataConfig['Please select an identifier.']) } const label = $('#select-id').find(`option[value='${data.name}']`).html(); if (data.value === '') { - return error('#id-errors', 'id-value', dataConfig['You need to give a value to ID.'].replace(/ID/, label)); + return error('#id-errors', '#id-value', dataConfig['You need to give a value to ID.'].replace(/ID/, label)); } if (['ocaid'].includes(data.name) && /\s/g.test(data.value)) { - return error('#id-errors', 'id-value', dataConfig['ID ids cannot contain whitespace.'].replace(/ID/, label)); + return error('#id-errors', '#id-value', dataConfig['ID ids cannot contain whitespace.'].replace(/ID/, label)); } let validId = true; @@ -214,12 +218,13 @@ export function validateIdentifiers(data) { if (isIdDupe(entries, data.value) === true) { // isbnOverride being set will override the dupe checker, so clear isbnOverride if there's a dupe. if (isbnOverride.get()) {isbnOverride.clear()} - return error('#id-errors', 'id-value', dataConfig['That ID already exists for this edition.'].replace(/ID/, label)); + return error('#id-errors', '#id-value', dataConfig['That ID already exists for this edition.'].replace(/ID/, label)); } if (validId === false) return false; $('#id-errors').hide(); + $('#select-id, #id-value').val(''); return true; } @@ -243,6 +248,7 @@ export function initClassificationValidation() { return error('#classification-errors', '#classification-value', dataConfig['You need to give a value to CLASS.'].replace(/CLASS/, label)); } $('#classification-errors').hide(); + $('#select-classification, #classification-value').val(''); return true; } }); @@ -394,6 +400,7 @@ export function initEditExcerpts() { return error('#excerpts-errors', '#excerpts-excerpt', 'That excerpt is too long.') } $('#excerpts-errors').hide(); + $('#excerpts-excerpt').val(''); return true; } }); @@ -443,6 +450,7 @@ export function initEditLinks() { return false; } $('#link-errors').addClass('hidden'); + $('#link-label, #link-url').val(''); return true; } }); From 20032e57af641221930ebb49f434b3768f34d922 Mon Sep 17 00:00:00 2001 From: Rebecca Shoptaw Date: Mon, 29 Jan 2024 15:36:48 -0500 Subject: [PATCH 3/6] Quick form HTML cleanup for work details page. --- openlibrary/templates/books/edit/excerpts.html | 2 +- openlibrary/templates/books/edit/web.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/openlibrary/templates/books/edit/excerpts.html b/openlibrary/templates/books/edit/excerpts.html index 2a0c4defedb..212571a52e5 100644 --- a/openlibrary/templates/books/edit/excerpts.html +++ b/openlibrary/templates/books/edit/excerpts.html @@ -1,6 +1,6 @@ $def with (work) -
- +
From cbba4ab438d18d9d38d6c81e5cfacc9d5eaf6f24 Mon Sep 17 00:00:00 2001 From: Rebecca Shoptaw Date: Tue, 30 Jan 2024 14:10:01 -0500 Subject: [PATCH 4/6] Add input clearing back to formdata, but only for identifiers. --- openlibrary/plugins/openlibrary/js/jquery.repeat.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/openlibrary/plugins/openlibrary/js/jquery.repeat.js b/openlibrary/plugins/openlibrary/js/jquery.repeat.js index 9537511ec38..3d4cf58e975 100644 --- a/openlibrary/plugins/openlibrary/js/jquery.repeat.js +++ b/openlibrary/plugins/openlibrary/js/jquery.repeat.js @@ -45,9 +45,15 @@ export default function($){ var data = {}; $(':input', elems.form).each(function() { var $e = $(this), - name = $e.attr('name'); + name = $e.attr('name'), + type = $e.attr('type'), + id = $e.attr('id'); data[name] = $e.val().trim(); + + if (type === 'text' && id === 'id-value') { + $e.val(''); + } }); return data; } From 924fa07a707871730940d535dab420bdc909620a Mon Sep 17 00:00:00 2001 From: Rebecca Shoptaw Date: Tue, 30 Jan 2024 14:12:00 -0500 Subject: [PATCH 5/6] Remove input clearing from the identifier functions,s but re-fill input in certain error cases. --- openlibrary/plugins/openlibrary/js/edit.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/openlibrary/plugins/openlibrary/js/edit.js b/openlibrary/plugins/openlibrary/js/edit.js index 8b9cbf41d55..7a7ea9ea91b 100644 --- a/openlibrary/plugins/openlibrary/js/edit.js +++ b/openlibrary/plugins/openlibrary/js/edit.js @@ -100,7 +100,6 @@ export function isbnConfirmAdd(data) { const noButtonSelector = '#do-not-add-isbn' const onYes = () => { $('#id-errors').hide(); - $('#select-id, #id-value').val(''); }; const onNo = () => { $('#id-errors').hide(); @@ -175,6 +174,7 @@ function validateLccn(data, dataConfig, label) { data.value = parseLccn(data.value); if (isValidLccn(data.value) === false) { + $('#id-value').val(data.value); return error('#id-errors', '#id-value', dataConfig['Invalid ID format'].replace(/ID/, label)); } return true; @@ -191,6 +191,7 @@ export function validateIdentifiers(data) { const dataConfig = JSON.parse(document.querySelector('#identifiers').dataset.config); if (data.name === '' || data.name === '---') { + $('#id-value').val(data.value); return error('#id-errors', '#select-id', dataConfig['Please select an identifier.']) } const label = $('#select-id').find(`option[value='${data.name}']`).html(); @@ -222,9 +223,7 @@ export function validateIdentifiers(data) { } if (validId === false) return false; - $('#id-errors').hide(); - $('#select-id, #id-value').val(''); return true; } From c4f799c97d0b054fdd0ff956bb719c55f883a7ef Mon Sep 17 00:00:00 2001 From: jimchamp Date: Thu, 22 Feb 2024 09:59:07 -0800 Subject: [PATCH 6/6] Apply suggestions from code review --- openlibrary/plugins/openlibrary/js/jquery.repeat.js | 4 ++-- openlibrary/templates/books/edit/web.html | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/openlibrary/plugins/openlibrary/js/jquery.repeat.js b/openlibrary/plugins/openlibrary/js/jquery.repeat.js index 3d4cf58e975..aec35c2d8be 100644 --- a/openlibrary/plugins/openlibrary/js/jquery.repeat.js +++ b/openlibrary/plugins/openlibrary/js/jquery.repeat.js @@ -47,11 +47,11 @@ export default function($){ var $e = $(this), name = $e.attr('name'), type = $e.attr('type'), - id = $e.attr('id'); + _id = $e.attr('id'); data[name] = $e.val().trim(); - if (type === 'text' && id === 'id-value') { + if (type === 'text' && _id === 'id-value') { $e.val(''); } }); diff --git a/openlibrary/templates/books/edit/web.html b/openlibrary/templates/books/edit/web.html index 7d10c3057e0..a1aaddfe81d 100644 --- a/openlibrary/templates/books/edit/web.html +++ b/openlibrary/templates/books/edit/web.html @@ -24,7 +24,7 @@
- +