Bug 35237 - Duplicate ids in markup of patron card layout edit form
Summary: Duplicate ids in markup of patron card layout edit form
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Harrison Hawkins
QA Contact: Marcel de Rooy
URL:
Keywords: Academy, accessibility
Depends on:
Blocks:
 
Reported: 2023-11-02 17:56 UTC by Owen Leonard
Modified: 2026-01-21 17:55 UTC (History)
6 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: Sponsored
Comma delimited list of Sponsors: Catalyst
Crowdfunding goal: 0
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
BUG: 35237 Duplicate ids in markup of patron card form updated to have unique IDs. (2.10 KB, patch)
2025-07-17 23:20 UTC, Courtney Brown
Details | Diff | Splinter Review
Bug 35237: Give patron card layout form elements unique IDs (5.91 KB, patch)
2025-07-20 22:09 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 35237: Give patron card layout form elements unique IDs (5.95 KB, patch)
2025-09-05 19:28 UTC, Noah Tremblay
Details | Diff | Splinter Review
Bug 35237: Changed labels above checkbox on the edit layout page. (3.24 KB, patch)
2026-01-21 02:11 UTC, Harrison Hawkins
Details | Diff | Splinter Review
Bug 35237: Give patron card layout form elements unique IDs (6.00 KB, patch)
2026-01-21 17:55 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 35237: Changed labels above checkbox on the edit layout page. (3.30 KB, patch)
2026-01-21 17:55 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 35237: (QA follow-up) Set label for attribute to first radio input (3.05 KB, patch)
2026-01-21 17:55 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2023-11-02 17:56:26 UTC
Tools -> Patron card creator -> Edit layout: 

A few pairs of radio buttons have duplicate labels:

<li>
   <label for="page_side">Page side: </label>
   <input type="radio" name="page_side" id="page_side" value="F">Front
   <input type="radio" name="page_side" id="page_side" value="B" checked="checked">Back
</li>

It would be better for the "Page side" label to have no "for" attribute and for  "Front" and "Back" to be labels.
Comment 1 Courtney Brown 2025-07-17 23:20:57 UTC
Created attachment 184253 [details] [review]
BUG: 35237 Duplicate ids in markup of patron card form updated to have unique IDs.

Test plan:
1. Head to 'tool's then 'patronn card creator'
2. Click on 'Layouts' underneath 'Manage'
3. Click on 'Edit'
4. Inspect the page and check that the ids for Page side are unique.
Comment 2 Aleisha Amohia 2025-07-20 22:09:10 UTC
Created attachment 184421 [details] [review]
Bug 35237: Give patron card layout form elements unique IDs

This patch ensures the following patron card layout form fields use unique IDs:
- page side
- guide box
- guide grid

This semantically links the options to their labels so is more accessible for screen readers and keyboard navigation.

The patch also adds a 'default' option to the original labels so they can become useful elements on the form rather than just text labels.

Test plan:
1. Head to 'tools' then 'patron card creator'
2. Click on 'Layouts' underneath 'Manage'
3. Click on 'Edit'
4. Confirm it's easy to tell which options are available and chosen for page side, guide box, and guide grid
5. Click on the text labels for the options i.e. front, back, on, off. Confirm the relevant radio button is selected

Sponsored-by: Catalyst IT
Comment 3 Noah Tremblay 2025-09-05 19:28:02 UTC
Created attachment 186246 [details] [review]
Bug 35237: Give patron card layout form elements unique IDs

This patch ensures the following patron card layout form fields use unique IDs:
- page side
- guide box
- guide grid

This semantically links the options to their labels so is more accessible for screen readers and keyboard navigation.

The patch also adds a 'default' option to the original labels so they can become useful elements on the form rather than just text labels.

Test plan:
1. Head to 'tools' then 'patron card creator'
2. Click on 'Layouts' underneath 'Manage'
3. Click on 'Edit'
4. Confirm it's easy to tell which options are available and chosen for page side, guide box, and guide grid
5. Click on the text labels for the options i.e. front, back, on, off. Confirm the relevant radio button is selected

Sponsored-by: Catalyst IT
Signed-off-by: noah <noah@inlibro.com>
Comment 4 Jonathan Druart 2025-09-16 09:29:33 UTC
Owen, can you confirm it's the our preferred styling for radio buttons?

Before: https://snipboard.io/CgRw6D.jpg

After:  https://snipboard.io/josfAn.jpg

I don't think the label's values should be bold.
Comment 5 Marcel de Rooy 2025-09-19 08:15:45 UTC
-                                        <label for="page_side">Page side: </label>
+                                        <label for="page_side_front">Page side: </label>
Not sure about this change. Why front here?

+                                            <label for="page_side_front">Front: <input type="radio" name="page_side" id="page_side_front" value="F" /></label>
Why include the input in the label ?

Please explain.
Comment 6 Harrison Hawkins 2026-01-21 02:11:32 UTC
Created attachment 191736 [details] [review]
Bug 35237: Changed labels above checkbox on the edit layout page.

Clarified the for attribute for the original labels of the checkboxes.

Test plan:
1. Head to 'tools' then 'patron card creator'
2. Click on 'Layouts' underneath 'Manage'
3. Click on 'Edit'
4. Confirm it's easy to tell which options are available and chosen for page side, guide box, and guide grid
5. Inspect the page, look at and confirm the original labels have been renamed for page size, guide box, guide grid

Sponsored-by: Catalyst IT
Comment 7 Harrison Hawkins 2026-01-21 02:16:43 UTC
(In reply to Marcel de Rooy from comment #5)
> -                                        <label for="page_side">Page side:
> </label>
> +                                        <label for="page_side_front">Page
> side: </label>
> Not sure about this change. Why front here?
> 
> +                                            <label
> for="page_side_front">Front: <input type="radio" name="page_side"
> id="page_side_front" value="F" /></label>
> Why include the input in the label ?
> 
> Please explain.

In response to your first comment we have changed the labels for page side, guide box, guide grid.

In response to second comment about inputs in labels we have noticed this formatting is elsewhere in koha for example see below:

koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt:                        <label><input type="radio" name="branch" value="homebranch" /> Home library</label>
Comment 8 Owen Leonard 2026-01-21 17:55:26 UTC
Created attachment 191806 [details] [review]
Bug 35237: Give patron card layout form elements unique IDs

This patch ensures the following patron card layout form fields use
unique IDs:
- page side
- guide box
- guide grid

This semantically links the options to their labels so is more
accessible for screen readers and keyboard navigation.

The patch also adds a 'default' option to the original labels so they
can become useful elements on the form rather than just text labels.

Test plan:
1. Head to 'tools' then 'patron card creator'
2. Click on 'Layouts' underneath 'Manage'
3. Click on 'Edit'
4. Confirm it's easy to tell which options are available and chosen for
   page side, guide box, and guide grid
5. Click on the text labels for the options i.e. front, back, on, off.
   Confirm the relevant radio button is selected

Sponsored-by: Catalyst IT
Signed-off-by: noah <noah@inlibro.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 9 Owen Leonard 2026-01-21 17:55:29 UTC
Created attachment 191807 [details] [review]
Bug 35237: Changed labels above checkbox on the edit layout page.

Clarified the for attribute for the original labels of the checkboxes.

Test plan:
1. Head to 'tools' then 'patron card creator'
2. Click on 'Layouts' underneath 'Manage'
3. Click on 'Edit'
4. Confirm it's easy to tell which options are available and chosen for
   page side, guide box, and guide grid
5. Inspect the page, look at and confirm the original labels have been
   renamed for page size, guide box, guide grid

Sponsored-by: Catalyst IT
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 10 Owen Leonard 2026-01-21 17:55:31 UTC
Created attachment 191808 [details] [review]
Bug 35237: (QA follow-up) Set label for attribute to first radio input

Labels with a "for" attribute should reference an existing input, so we
should keep the "Page side," "Guide box," and "Guide grid" for attribute
pointing to the first corresponding radio input in that row.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>