The markup for the header search forms in the staff interface use a <p> tag to show descriptive text about the form. For instance: <p class="tip">Enter patron card number or partial name:</p> In most cases this can be replaced with a label: <label class="tip" for="findborrower">Enter patron card number or partial name:</label> This will improve accessibility and eliminate errors raised by WCAG checks.
Created attachment 106659 [details] [review] Bug 25954: Replace paragraph tags with labels in header search forms This patch replaces instances of <p class="tip"> with <label class="tip"> in header search forms where there is not already a label tag corresponding to the first form field. In cases where there is already a label on the first form field the <p> is kept. To test, apply the patch and rebuild the staff client CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) Test the header search forms on the following pages: - Staff interface home page - Acquisitions home - Acquisitions -> Vendor -> Contracts - Acquisitions -> Vendor -> Add to basket -> From a suggestion - Advanced catalog search - Authorities - Administration - Administration -> Funds - Administration -> Desks (UseCirculationDesks must be enabled) - Administration -> Cities and towns - Administration -> Currencies and exchange rates - Administration -> Patron categories - Administration -> Z39.50/SRU servers - Catalog -> Search results - Cataloging - Circulation - Circulation -> Check in - Patrons - Serials - Tools -> Notices and Slips In each case, check that the search form labels work as expected to move focus to the corresponding form field.
Created attachment 107896 [details] [review] Bug 25954: Replace paragraph tags with labels in header search forms This patch replaces instances of <p class="tip"> with <label class="tip"> in header search forms where there is not already a label tag corresponding to the first form field. In cases where there is already a label on the first form field the <p> is kept. To test, apply the patch and rebuild the staff client CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) Test the header search forms on the following pages: - Staff interface home page - Acquisitions home - Acquisitions -> Vendor -> Contracts - Acquisitions -> Vendor -> Add to basket -> From a suggestion - Advanced catalog search - Authorities - Administration - Administration -> Funds - Administration -> Desks (UseCirculationDesks must be enabled) - Administration -> Cities and towns - Administration -> Currencies and exchange rates - Administration -> Patron categories - Administration -> Z39.50/SRU servers - Catalog -> Search results - Cataloging - Circulation - Circulation -> Check in - Patrons - Serials - Tools -> Notices and Slips In each case, check that the search form labels work as expected to move focus to the corresponding form field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Excellent work, all looks good to me.. Signing off
Comment on attachment 107896 [details] [review] Bug 25954: Replace paragraph tags with labels in header search forms Review of attachment 107896 [details] [review]: ----------------------------------------------------------------- ::: koha-tmpl/intranet-tmpl/prog/en/includes/cat-search.inc @@ +28,3 @@ > <form action="/cgi-bin/koha/catalogue/search.pl" method="get" id="cat-search-block"> > + [% IF ( Koha.Preference('IntranetCatalogSearchPulldown') ) %] > + <label class="tip" for="idx">Enter search keywords:</label> I am not sure about for="idx", I think the label is still for "search-form" not for indices select...
Created attachment 107900 [details] [review] Bug 25954: Replace paragraph tags with labels in header search forms This patch replaces instances of <p class="tip"> with <label class="tip"> in header search forms where there is not already a label tag corresponding to the first form field. In cases where there is already a label on the first form field the <p> is kept. To test, apply the patch and rebuild the staff client CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) Test the header search forms on the following pages: - Staff interface home page - Acquisitions home - Acquisitions -> Vendor -> Contracts - Acquisitions -> Vendor -> Add to basket -> From a suggestion - Advanced catalog search - Authorities - Administration - Administration -> Funds - Administration -> Desks (UseCirculationDesks must be enabled) - Administration -> Cities and towns - Administration -> Currencies and exchange rates - Administration -> Patron categories - Administration -> Z39.50/SRU servers - Catalog -> Search results - Cataloging - Circulation - Circulation -> Check in - Patrons - Serials - Tools -> Notices and Slips In each case, check that the search form labels work as expected to move focus to the corresponding form field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Ignore my sign off, I tested this and it all looks good to me, I ran each page through a WCAG checker. I did not notice Josef's comment.
Created attachment 107915 [details] [review] Bug 25954: (Follow-up) Correct label "for" attribute This patch updates a couple of search include forms to change the "for" attribute of the label. Although "idx" is the first field in this situation, the label's text applies to the keyword field so it should give focus to the keyword field.
Comment on attachment 107915 [details] [review] Bug 25954: (Follow-up) Correct label "for" attribute Review of attachment 107915 [details] [review]: ----------------------------------------------------------------- Sorry being so pedantive... ::: koha-tmpl/intranet-tmpl/prog/en/includes/cat-search.inc @@ +27,4 @@ > <div id="catalog_search" class="residentsearch"> > <form action="/cgi-bin/koha/catalogue/search.pl" method="get" id="cat-search-block"> > [% IF ( Koha.Preference('IntranetCatalogSearchPulldown') ) %] > + <label class="tip" for="search-form">Enter search keywords:</label> The label could be before IF and you do not need the ELSE branch then... also there are some more occurences, see: git grep 'for="idx"'
So it need probably more refactoring... whole form with id "cat-search-block" could be in its own template?
(In reply to Josef Moravec from comment #9) > So it need probably more refactoring... This patch was what prompted me to start the conversation on koha-devel about consolidating the staff interface header search forms.
(In reply to Owen Leonard from comment #10) > (In reply to Josef Moravec from comment #9) > > So it need probably more refactoring... > > This patch was what prompted me to start the conversation on koha-devel > about consolidating the staff interface header search forms. I still like the idea - because it means we have less files to change something when needed.
(In reply to Owen Leonard from comment #10) > (In reply to Josef Moravec from comment #9) > > So it need probably more refactoring... > > This patch was what prompted me to start the conversation on koha-devel > about consolidating the staff interface header search forms. I see, sorry I missed the whole conversation on koha-devel So, I think we could go with this patches and refactoring on another bug report? Looks like, there is no clear consensus on that conversation. I do not have strong opinion on that other that it needs to be refactored somehow... But I could imagine one big file, or WRAPPER (as mantioned by Julian) approach...
Created attachment 107986 [details] [review] Bug 25954: (Follow-up) Correct label "for" attribute This patch updates a couple of search include forms to change the "for" attribute of the label. Although "idx" is the first field in this situation, the label's text applies to the keyword field so it should give focus to the keyword field.
With the patch applied some of the labels jump in front of the search field while others remain above, an example would be the administration module. I did run yarn build and restart_all.
(In reply to Katrin Fischer from comment #14) > With the patch applied some of the labels jump in front of the search field > while others remain above Could you look at this again? The problem you describe would happen if the CSS hasn't been regenerated. I know you said you did, but I'm hoping it was a glitch.
Created attachment 113747 [details] [review] Bug 25954: Replace paragraph tags with labels in header search forms This patch replaces instances of <p class="tip"> with <label class="tip"> in header search forms where there is not already a label tag corresponding to the first form field. In cases where there is already a label on the first form field the <p> is kept. To test, apply the patch and rebuild the staff client CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) Test the header search forms on the following pages: - Staff interface home page - Acquisitions home - Acquisitions -> Vendor -> Contracts - Acquisitions -> Vendor -> Add to basket -> From a suggestion - Advanced catalog search - Authorities - Administration - Administration -> Funds - Administration -> Desks (UseCirculationDesks must be enabled) - Administration -> Cities and towns - Administration -> Currencies and exchange rates - Administration -> Patron categories - Administration -> Z39.50/SRU servers - Catalog -> Search results - Cataloging - Circulation - Circulation -> Check in - Patrons - Serials - Tools -> Notices and Slips In each case, check that the search form labels work as expected to move focus to the corresponding form field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 113748 [details] [review] Bug 25954: (follow-up) Correct label "for" attribute This patch updates a couple of search include forms to change the "for" attribute of the label. Although "idx" is the first field in this situation, the label's text applies to the keyword field so it should give focus to the keyword field. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 113749 [details] [review] Bug 25954: (QA follow-up) Correct label "for" attribute in authorities search Change it from first select box to text input for consistancy with other search forms Test plan: 1) Apply the patch 2) Go to authorities search 3) Confirm the labels in all search tabs are linked to text input field Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
I would consider it PQA, but would like to somebody to look into my follow-up to ensure it does make sense.
Created attachment 114681 [details] [review] Bug 25954: Replace paragraph tags with labels in header search forms This patch replaces instances of <p class="tip"> with <label class="tip"> in header search forms where there is not already a label tag corresponding to the first form field. In cases where there is already a label on the first form field the <p> is kept. To test, apply the patch and rebuild the staff client CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) Test the header search forms on the following pages: - Staff interface home page - Acquisitions home - Acquisitions -> Vendor -> Contracts - Acquisitions -> Vendor -> Add to basket -> From a suggestion - Advanced catalog search - Authorities - Administration - Administration -> Funds - Administration -> Desks (UseCirculationDesks must be enabled) - Administration -> Cities and towns - Administration -> Currencies and exchange rates - Administration -> Patron categories - Administration -> Z39.50/SRU servers - Catalog -> Search results - Cataloging - Circulation - Circulation -> Check in - Patrons - Serials - Tools -> Notices and Slips In each case, check that the search form labels work as expected to move focus to the corresponding form field. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 114682 [details] [review] Bug 25954: (follow-up) Correct label "for" attribute This patch updates a couple of search include forms to change the "for" attribute of the label. Although "idx" is the first field in this situation, the label's text applies to the keyword field so it should give focus to the keyword field. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 114683 [details] [review] Bug 25954: (QA follow-up) Correct label "for" attribute in authorities search Change it from first select box to text input for consistancy with other search forms Test plan: 1) Apply the patch 2) Go to authorities search 3) Confirm the labels in all search tabs are linked to text input field Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Pushed to master for 21.05, thanks to everybody involved!
Owen, the following changes is causing t/db_dependent/selenium/administration_tasks.t to fail: diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/patrons-admin-search.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/patrons-admin-search.inc index a77b92c6d5..8640c2268e 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/patrons-admin-search.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/patrons-admin-search.inc @@ -3,18 +3,22 @@ <h1 id="logo"><a href="/cgi-bin/koha/mainpage.pl">[% LibraryName | html %]</a></h1><!-- Begin Patrons Admin Resident Search Box --> <div id="header_search"> <div id="patron_category_search" class="residentsearch"> - <p class="tip">Search by patron category name:</p> <form action="[% script_name | html %]" method="post"> - <input class="head-searchbox" type="text" name="description" size="40" value="[% searchfield | html %]" /> + <label class="tip" for="description">Search by patron category name:</label> + <input class="head-searchbox" type="text" name="description" id="description" size="40" value="[% searchfield | html %]" /> There are now 2 elements with id="description" in the DOM.
The following patch would make the tests pass again. - <input class="head-searchbox" type="text" name="description" id="description" size="40" value="[% searchfield | html %]" /> + <input class="head-searchbox" type="text" name="description" size="40" value="[% searchfield | html %]" />
Created attachment 114790 [details] [review] Bug 25954: Prevent two elements with id="description" in the DOM It also fixes t/db_dependent/selenium/administration_tasks.t
(In reply to Jonathan Druart from comment #26) > Created attachment 114790 [details] [review] [review] > Bug 25954: Prevent two elements with id="description" in the DOM > > It also fixes t/db_dependent/selenium/administration_tasks.t Pushed to master for 21.05.00
Pushed to 20.11.x for 20.11.02
Note that there is no change in appearance.
Pushed to 20.08.x for 20.05.08
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.