Bug 37373 - Cursor should go to patron search box on loading holds page
Summary: Cursor should go to patron search box on loading holds page
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Owen Leonard
QA Contact: Lucas Gass
URL:
Keywords: additional_work_needed
Depends on:
Blocks:
 
Reported: 2024-07-16 15:38 UTC by Andrew Fuerste-Henry
Modified: 2024-08-29 17:39 UTC (History)
3 users (show)

See Also:
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 (1.35 KB, patch)
2024-07-25 22:25 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 37373: [Alternate] Combine duplicate class attributes (2.46 KB, patch)
2024-07-26 16:46 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 37373: If search_patron_filter is present set focus on it (1.41 KB, patch)
2024-07-26 18:29 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 37373: [Alternate] Combine duplicate class attributes (2.52 KB, patch)
2024-07-26 18:29 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 37373: [Alternate] Combine duplicate class attributes (2.57 KB, patch)
2024-07-26 20:26 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 37373: (follow-up) Add id and adjust tests (8.02 KB, patch)
2024-08-07 17:37 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2024-07-16 15:38:48 UTC
In 23.11 and previous, Koha put the user's cursor into the patron search field upon loading the screen on which to search for a patron to place a hold on a title. In 24.05 and later, it puts the cursor instead into the Checkout field at the top of the page. This is not a desirable change.

To reproduce:
 - Go to a bib record (detail.pl)
 - click Holds on the left or Place a Hold above the title
 - note that on the next page (request.pl) your cursor is not in the box to search for a patron to place a hold
Comment 1 Lucas Gass 2024-07-25 21:57:14 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.
Comment 2 Lucas Gass 2024-07-25 22:25:54 UTC Comment hidden (obsolete)
Comment 3 Lucas Gass 2024-07-25 22:29:05 UTC
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.
Comment 4 Andrew Fuerste-Henry 2024-07-26 16:29:19 UTC
(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.
Comment 5 Owen Leonard 2024-07-26 16:46:23 UTC Comment hidden (obsolete)
Comment 6 Andrew Fuerste-Henry 2024-07-26 18:29:43 UTC Comment hidden (obsolete)
Comment 7 Andrew Fuerste-Henry 2024-07-26 18:29:45 UTC Comment hidden (obsolete)
Comment 8 Andrew Fuerste-Henry 2024-07-26 18:34:24 UTC
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.
Comment 9 Owen Leonard 2024-07-26 18:38:22 UTC
No offense to Lucas but if my patch works his should not be needed.
Comment 10 Andrew Fuerste-Henry 2024-07-26 18:47:16 UTC
(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.
Comment 11 Lucas Gass 2024-07-26 20:24:55 UTC
(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!
Comment 12 Lucas Gass 2024-07-26 20:26:27 UTC
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>
Comment 13 Katrin Fischer 2024-08-01 16:27:23 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 14 Katrin Fischer 2024-08-02 14:55:06 UTC
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".
Comment 15 Andrew Fuerste-Henry 2024-08-07 16:03:34 UTC
Can we please get this backported to 24.05?
Comment 16 Owen Leonard 2024-08-07 16:15:03 UTC
(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.
Comment 17 Nick Clemens (kidclamp) 2024-08-07 17:37:39 UTC
Created attachment 170145 [details] [review]
Bug 37373: (follow-up) Add id and adjust tests
Comment 18 Katrin Fischer 2024-08-08 12:37:37 UTC
Thanks for the help, Nick. Pushed to main (shortly).
Comment 19 Lucas Gass 2024-08-29 17:39:08 UTC
Backported to 24.05.x for upcoming 24.05.04