| Summary: | Cursor should go to patron search box on loading holds page | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Andrew Fuerste-Henry <andrew> |
| Component: | Hold requests | Assignee: | Owen Leonard <oleonard> |
| Status: | Pushed to stable --- | QA Contact: | Lucas Gass (lukeg) <lucas> |
| Severity: | minor | ||
| Priority: | P5 - low | CC: | clackman, fridolin.somers, gmcharlt, kebliss, lucas |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
24.11.00,24.05.04
|
|
| Circulation function: | |||
| Attachments: |
Bug 37373: If search_patron_filter is present set focus on it
Bug 37373: [Alternate] Combine duplicate class attributes Bug 37373: If search_patron_filter is present set focus on it Bug 37373: [Alternate] Combine duplicate class attributes Bug 37373: [Alternate] Combine duplicate class attributes Bug 37373: (follow-up) Add id and adjust tests |
||
|
Description
Andrew Fuerste-Henry
2024-07-16 15:38:48 UTC
If you look closely as that page loads you can see that it initially has focus but focus is immediately given to the Check out input. Created attachment 169616 [details] [review] Bug 37373: If search_patron_filter is present set focus on it To test: 1- Go to a bib record (detail.pl) 2- click Holds on the left or Place a Hold above the title 3- note that on the next page (request.pl) your cursor is not in the box to search for a patron to place a hold 4- APPLY PATCH and clear browser cache 5- Try again, focus should be set on the patron search field This patch will change the focus to the search_patron_filter element whenever it is present, so that is more pages that just request.tt: It will also do so anywhere where we call patron-search.inc: koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt:[% PROCESS 'patron-search.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt:[% PROCESS 'patron-search.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt:[% PROCESS 'patron-search.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/circ/request-article.tt:[% PROCESS "patron-search.inc" %] koha-tmpl/intranet-tmpl/prog/en/modules/erm/erm.tt:[% PROCESS 'patron-search.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/member.tt:[% PROCESS 'patron-search.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt:[% PROCESS 'patron-search.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/patroncards/edit-batch.tt:[% PROCESS 'patron-search.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt:[% PROCESS "patron-search.inc" %] koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt: [% SET search_results_block_id = 'holds_patronsearch_pane_panel' %] [%# adjusting variable for patron-search.inc %] koha-tmpl/intranet-tmpl/prog/en/modules/serials/routing.tt:[% PROCESS 'patron-search.inc' %] koha-tmpl/intranet-tmpl/prog/en/modules/suggestion/suggestion.tt:[% PROCESS 'patron-search.inc' %] I do think that makes the behavior act closer to what it was in 23.11 and below. (In reply to Lucas Gass from comment #3) I went through several of these to track down where they are and how this patch impacts them. I didn't make it through the full list before deciding to fail QA on this patch. > koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt:[% PROCESS > 'patron-search.inc' %] This is the interface to set the manager of a basket. This patch does change behavior here (does not cause the cursor to default into the one of the patron search fields) and results in an error: Blocked aria-hidden on a <div> element because the element that just received focus must not be hidden from assistive technology users. Avoid using aria-hidden on a focused element or its ancestor. Consider using the inert attribute instead, which will also prevent focus. For more details, see the aria-hidden section of the WAI-ARIA specification at https://w3c.github.io/aria/#aria-hidden. > koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt:[% PROCESS > 'patron-search.inc' %] This is the interface to select a patron to notify upon receiving an order. This patch does not change focus behavior on the page (there is no cursor when the page loads) but does not produce the same error as above. > koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgets.tt:[% PROCESS > 'patron-search.inc' %] This is the form to define the owner of a fund. This patch does not change focus behavior on the page (there is no cursor when the page loads) but does not produce the same error as above. > koha-tmpl/intranet-tmpl/prog/en/modules/circ/request-article.tt:[% PROCESS > "patron-search.inc" %] This is the interface to select a patron for whom to enter an article request via the intranet, linked off the bib just like placing a hold. This patch correctly puts the cursor in the patron search box. > koha-tmpl/intranet-tmpl/prog/en/modules/erm/erm.tt:[% PROCESS > 'patron-search.inc' %] This seems to be the Select User modal that's used by several parts of the ERM module. This patch does not change the focus behavior on the form and produces the aria-hidden error I pasted above. Created attachment 169764 [details] [review] Bug 37373: [Alternate] Combine duplicate class attributes This patch corrects two instances in patron-search.inc where there were two class attributes on one input. Combining the two class names under one class attribute seems to fix the focus problem. The patch also updates the global JS giving focus to elements with a "focus" class so that it only targets elements which are visible. This prevents the browser from trying to put focus on a field in a hidden modal. Created attachment 169770 [details] [review] Bug 37373: If search_patron_filter is present set focus on it To test: 1- Go to a bib record (detail.pl) 2- click Holds on the left or Place a Hold above the title 3- note that on the next page (request.pl) your cursor is not in the box to search for a patron to place a hold 4- APPLY PATCH and clear browser cache 5- Try again, focus should be set on the patron search field Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Created attachment 169771 [details] [review] Bug 37373: [Alternate] Combine duplicate class attributes This patch corrects two instances in patron-search.inc where there were two class attributes on one input. Combining the two class names under one class attribute seems to fix the focus problem. The patch also updates the global JS giving focus to elements with a "focus" class so that it only targets elements which are visible. This prevents the browser from trying to put focus on a field in a hidden modal. Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> With these patches, the focus gets set on the patron search form as desired when loading the patron search pages while placing either a hold or article request. The second patch clears the aria-hidden error message I mentioned previously. I don't see any change in focus behavior on the various acquisitions pages I mentioned in my previous comment. No offense to Lucas but if my patch works his should not be needed. (In reply to Owen Leonard from comment #9) > No offense to Lucas but if my patch works his should not be needed. In retrospect, I coulda figured out that's what "alternate" means. I just re-tested with only the alternate patch attached and confirmed that it works as desired. (In reply to Owen Leonard from comment #9) > No offense to Lucas but if my patch works his should not be needed. No offense taken! Glad we got the right fix here, thanks Owen! Created attachment 169790 [details] [review] Bug 37373: [Alternate] Combine duplicate class attributes This patch corrects two instances in patron-search.inc where there were two class attributes on one input. Combining the two class names under one class attribute seems to fix the focus problem. The patch also updates the global JS giving focus to elements with a "focus" class so that it only targets elements which are visible. This prevents the browser from trying to put focus on a field in a hidden modal. Signed-off-by: Andrew Fuerste Henry <andrewfh@dubcolib.org> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Pushed for 24.11! Well done everyone, thank you! I am sorry, but this seems to break the tests, please fix: # at t/db_dependent/selenium/patrons_search.t line 464. Error while executing command: no such element: Unable to locate element: //*[@class="search_patron_filter"] at /usr/share/perl5/Selenium/Remote/Driver.pm line 411. at /usr/share/perl5/Selenium/Remote/Driver.pm line 356. # Looks like your test exited with 255 just after 1. This is where we added the "focus". Can we please get this backported to 24.05? (In reply to Andrew Fuerste-Henry from comment #15) > Can we please get this backported to 24.05? Backporting as-is will break tests in 24.05 too, so I'm assuming we want to find a fix for the test first? Sorry, fixing the test is over my head. Created attachment 170145 [details] [review] Bug 37373: (follow-up) Add id and adjust tests Thanks for the help, Nick. Pushed to main (shortly). Backported to 24.05.x for upcoming 24.05.04 additional_work_needed can be removed ? (In reply to Fridolin Somers from comment #20) > additional_work_needed can be removed ? Yes |