This issue came up because I noticed that the tag review page's "test" feature wasn't working. Clicking the test button didn't trigger any message about the tag's approval status. $(input).attr("value") is getting the initial contents of the "value" attribute when the DOM initially loads. In the case of the tags page, this will always be an empty string. Using $(input).val() gets the current value of the field. I think other instances of this code aren't buggy in the same way, but I think it's worth updating all of them for consistency.
Created attachment 146444 [details] [review] Bug 32933: Use val() instead of attr("value") when getting field values with jQuery This patch replaces instances in Koha's templates where "attr('value')" is used to get the value of a form field, using "val()" instead. This ensures that we get the current value of the field, not just the value of the field when the page first loaded. To test, apply the patch and test the following pages: - Tools -> Tags. Use the "Check lists" form in the right-hand sidebar to test the approval status of some approved and rejected tags. It should correctly respond with the tag's status. - Cataloging -> Label creator. - New -> Printer profile. Each offset and creep field should correctly reflect the selected value in the "Units" dropdown. Changing the dropdown should make the units change accordingly. - New -> Label template. Perform the same tests as above. - Tools -> Patron card creator. - New -> Layout. If necessary, check the "Field 1" checkbox. The units should be displayed correctly and change as you change the "Units" dropdown. - New -> Printer profile, - New -> Card template: Perform the same tests as above. - Lists -> List contents. - Check that the "Add to" dropdown menu works for adding selected titles to a cart or another list. - Back record modification. Submit a batch of records for modification and go to Administration -> Manage jobs. - View the details of the job you just submitted. - Confirm that the "Add modified records to the following list" dropdown works correctly. - Cataloging -> Find a bibliographic record -> Edit items. - Confirm that clicking a cell in the table of existing items triggers the "Edit item/Delete item" menu, and that these links work correctly.
*** Bug 32936 has been marked as a duplicate of this bug. ***
Created attachment 146601 [details] [review] Bug 32933: Use val() instead of attr("value") when getting field values with jQuery This patch replaces instances in Koha's templates where "attr('value')" is used to get the value of a form field, using "val()" instead. This ensures that we get the current value of the field, not just the value of the field when the page first loaded. To test, apply the patch and test the following pages: - Tools -> Tags. Use the "Check lists" form in the right-hand sidebar to test the approval status of some approved and rejected tags. It should correctly respond with the tag's status. - Cataloging -> Label creator. - New -> Printer profile. Each offset and creep field should correctly reflect the selected value in the "Units" dropdown. Changing the dropdown should make the units change accordingly. - New -> Label template. Perform the same tests as above. - Tools -> Patron card creator. - New -> Layout. If necessary, check the "Field 1" checkbox. The units should be displayed correctly and change as you change the "Units" dropdown. - New -> Printer profile, - New -> Card template: Perform the same tests as above. - Lists -> List contents. - Check that the "Add to" dropdown menu works for adding selected titles to a cart or another list. - Back record modification. Submit a batch of records for modification and go to Administration -> Manage jobs. - View the details of the job you just submitted. - Confirm that the "Add modified records to the following list" dropdown works correctly. - Cataloging -> Find a bibliographic record -> Edit items. - Confirm that clicking a cell in the table of existing items triggers the "Edit item/Delete item" menu, and that these links work correctly. Works as described. Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Hi Owen, I have a question from testing this patch, when I do: 1. Navigate to: Tools -> Patron card creator > New -> Layout: 2. Check "Field 1" checkbox 3. Change the value in the "Units" dropdown 4. I observed that the unit beside the 'Font size' input box does not get updated - it stays as 'pt'. My question is should the "Font size" unit also update given #font_size is listed here: https://github.com/Koha-Community/Koha/blob/178e40ae9fd7246fa638a64c34fde06f94548ff0/koha-tmpl/intranet-tmpl/prog/en/modules/patroncards/edit-layout.tt#L503
(In reply to Alex Buckley from comment #4) > My question is should the "Font size" unit also update given #font_size is > listed here I assumed based on the behavior in master that changing the "global" units selection should not affect the font size inputs. The "#font_size" ID doesn't exist on its own in the template, which makes me wonder if the entry is a relic of a previous version.
(In reply to Owen Leonard from comment #5) > (In reply to Alex Buckley from comment #4) > > My question is should the "Font size" unit also update given #font_size is > > listed here > > I assumed based on the behavior in master that changing the "global" units > selection should not affect the font size inputs. The "#font_size" ID > doesn't exist on its own in the template, which makes me wonder if the entry > is a relic of a previous version. Yes, that sounds fair enough. Thank you Owen.
> I think other instances of this code aren't buggy in the same way, but I > think it's worth updating all of them for consistency. Should these (or some of these) be fixed as well? koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc-overlay-rules.tt: .attr('value', op) koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc-overlay-rules.tt: .attr('value', id) koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc-overlay-rules.tt: doSubmit('doedit', $("#doedit").attr('value')); koha-tmpl/intranet-tmpl/prog/en/modules/admin/oai_set_mappings.tt: tr.find('input[type="text"]').attr("value",""); koha-tmpl/intranet-tmpl/prog/en/modules/admin/systempreferences.tt: $('#preftype').attr("value",$(this).attr("id")); koha-tmpl/intranet-tmpl/prog/en/modules/authorities/authorities.tt: $("#confirm_not_duplicate").attr("value","1"); koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt: opt.attr("value", filter.search_filter_id); koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/search-history.tt: .attr('value', $(this).attr('value')); koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt: $("#redirect").attr("value",dest); koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt: $("#confirm_not_duplicate").attr("value","1"); koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt: $("#redirect").attr("value",redirect); koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt: let amount_outstanding = parseFloat( $('#amountoutstanding').attr('value') ); koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt: let amount_writeoff = parseFloat( $('#amountwrittenoff').attr('value') ); koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt: .attr('value', action) koha-tmpl/intranet-tmpl/prog/en/modules/tools/stage-marc-import.tt: opt.attr("value", profile.profile_id); koha-tmpl/intranet-tmpl/prog/js/members.js: $("input#patron_attr_" + newId, clone).attr('value',''); koha-tmpl/intranet-tmpl/prog/js/members.js: $("select#patron_attr_" + newId, clone).attr('value',''); koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: .find('textarea').attr('value', '').end() koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: .find('select').attr('value', '').end(); koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: $('input[type!="hidden"],select,textarea', clone).attr('id', 'patron-attr-' + newId).attr('value', ''); koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: $("label", clone).attr('for', 'patron-attr-' + newId).attr('value', ''); koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt: $("#hold_form_biblios").attr("value", numList); I used: git grep "\.attr(.v"
Created attachment 146978 [details] [review] Bug 32933: Use val() instead of attr("value") when getting field values with jQuery This patch replaces instances in Koha's templates where "attr('value')" is used to get the value of a form field, using "val()" instead. This ensures that we get the current value of the field, not just the value of the field when the page first loaded. To test, apply the patch and test the following pages: - Tools -> Tags. Use the "Check lists" form in the right-hand sidebar to test the approval status of some approved and rejected tags. It should correctly respond with the tag's status. - Cataloging -> Label creator. - New -> Printer profile. Each offset and creep field should correctly reflect the selected value in the "Units" dropdown. Changing the dropdown should make the units change accordingly. - New -> Label template. Perform the same tests as above. - Tools -> Patron card creator. - New -> Layout. If necessary, check the "Field 1" checkbox. The units should be displayed correctly and change as you change the "Units" dropdown. - New -> Printer profile, - New -> Card template: Perform the same tests as above. - Lists -> List contents. - Check that the "Add to" dropdown menu works for adding selected titles to a cart or another list. - Back record modification. Submit a batch of records for modification and go to Administration -> Manage jobs. - View the details of the job you just submitted. - Confirm that the "Add modified records to the following list" dropdown works correctly. - Cataloging -> Find a bibliographic record -> Edit items. - Confirm that clicking a cell in the table of existing items triggers the "Edit item/Delete item" menu, and that these links work correctly. Works as described. Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Decided to not hold this one up, as it has nice test plans and all that. Owen, could you please verify if we should have a 'part 2' type of bug to fix the other occurences? > > > I think other instances of this code aren't buggy in the same way, but I > > think it's worth updating all of them for consistency. > > Should these (or some of these) be fixed as well? > > > koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc-overlay-rules.tt: > .attr('value', op) > koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc-overlay-rules.tt: > .attr('value', id) > koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc-overlay-rules.tt: > doSubmit('doedit', $("#doedit").attr('value')); > koha-tmpl/intranet-tmpl/prog/en/modules/admin/oai_set_mappings.tt: > tr.find('input[type="text"]').attr("value",""); > koha-tmpl/intranet-tmpl/prog/en/modules/admin/systempreferences.tt: > $('#preftype').attr("value",$(this).attr("id")); > koha-tmpl/intranet-tmpl/prog/en/modules/authorities/authorities.tt: > $("#confirm_not_duplicate").attr("value","1"); > koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/results.tt: > opt.attr("value", filter.search_filter_id); > koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/search-history.tt: > .attr('value', $(this).attr('value')); > koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt: > $("#redirect").attr("value",dest); > koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt: > $("#confirm_not_duplicate").attr("value","1"); > koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt: > $("#redirect").attr("value",redirect); > koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt: > let amount_outstanding = parseFloat( $('#amountoutstanding').attr('value') ); > koha-tmpl/intranet-tmpl/prog/en/modules/members/paycollect.tt: > let amount_writeoff = parseFloat( $('#amountwrittenoff').attr('value') ); > koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt: > .attr('value', action) > koha-tmpl/intranet-tmpl/prog/en/modules/tools/stage-marc-import.tt: > opt.attr("value", profile.profile_id); > koha-tmpl/intranet-tmpl/prog/js/members.js: $("input#patron_attr_" + > newId, clone).attr('value',''); > koha-tmpl/intranet-tmpl/prog/js/members.js: $("select#patron_attr_" + > newId, clone).attr('value',''); > koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: > .find('textarea').attr('value', '').end() > koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: > .find('select').attr('value', '').end(); > koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: > $('input[type!="hidden"],select,textarea', clone).attr('id', 'patron-attr-' > + newId).attr('value', ''); > koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt: > $("label", clone).attr('for', 'patron-attr-' + newId).attr('value', ''); > koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt: > $("#hold_form_biblios").attr("value", numList); > > I used: git grep "\.attr(.v"
Pushed to master for 23.05. Nice work everyone, thanks!
Nice work, thanks everyone! Pushed to 22.11.x for the next release.
Lots of merge conflicts here with 22.05.x, no backport.
Created attachment 147605 [details] [review] Bug 32933: [22.05.x] Use val() instead of attr("value") when getting field values with jQuery This patch replaces instances in Koha's templates where "attr('value')" is used to get the value of a form field, using "val()" instead. This ensures that we get the current value of the field, not just the value of the field when the page first loaded. To test, apply the patch and test the following pages: - Tools -> Tags. Use the "Check lists" form in the right-hand sidebar to test the approval status of some approved and rejected tags. It should correctly respond with the tag's status. - Cataloging -> Label creator. - New -> Printer profile. Each offset and creep field should correctly reflect the selected value in the "Units" dropdown. Changing the dropdown should make the units change accordingly. - New -> Label template. Perform the same tests as above. - Tools -> Patron card creator. - New -> Layout. If necessary, check the "Field 1" checkbox. The units should be displayed correctly and change as you change the "Units" dropdown. - New -> Printer profile, - New -> Card template: Perform the same tests as above. - Lists -> List contents. - Check that the "Add to" dropdown menu works for adding selected titles to a cart or another list. - Back record modification. Submit a batch of records for modification and go to Administration -> Manage jobs. - View the details of the job you just submitted. - Confirm that the "Add modified records to the following list" dropdown works correctly. - Cataloging -> Find a bibliographic record -> Edit items. - Confirm that clicking a cell in the table of existing items triggers the "Edit item/Delete item" menu, and that these links work correctly. Works as described. Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
I rebased this for 22.05.x because the issue that originally prompted the patch (using the "Check lists" form in tags) exists in that version too.
Thanks Owen! Backported to 22.05.x for upcoming 22.05.11
applied to 21.11.x for 21.11.19
Not backported to 21.05.x