Bug 37748

Summary: In Bootstrap 5 "disabled" class must be on anchor tag, not list item
Product: Koha Reporter: Owen Leonard <oleonard>
Component: TemplatesAssignee: Owen Leonard <oleonard>
Status: Pushed to main --- QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: david, jonathan.druart, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00
Circulation function:
Bug Depends on: 35402    
Bug Blocks: 38049    
Attachments: Bug 37748: In Bootstrap 5 "disabled" class must be on anchor tag, not list item
Bug 37748: In Bootstrap 5 "disabled" class must be on anchor tag, not list item
Bug 37748: (follow-up) Move tooltip initialization to staff-global.js
Bug 37748: Adjust the 'Update child to patron' selenium test
Bug 37748: (follow-up) Move tooltip initialization to staff-global.js
Bug 37748: In Bootstrap 5 "disabled" class must be on anchor tag, not list item
Bug 37748: Adjust the 'Update child to patron' selenium test
Bug 37748: (follow-up) Move tooltip initialization to staff-global.js
Bug 37748: In Bootstrap 5 "disabled" class must be on anchor tag, not list item
Bug 37748: Adjust the 'Update child to patron' selenium test
Bug 37748: (follow-up) Move tooltip initialization to staff-global.js

Description Owen Leonard 2024-08-27 11:19:34 UTC
In Bootstrap 3.x we used <li class="disabled"> for a disabled dropdown menu list item, but now the class should be on the anchor tag inside the <li>
Comment 1 Owen Leonard 2024-08-27 20:19:45 UTC Comment hidden (obsolete)
Comment 2 David Nind 2024-08-28 01:51:25 UTC
Created attachment 170793 [details] [review]
Bug 37748: In Bootstrap 5 "disabled" class must be on anchor tag, not list item

This patch revises the markup for disabled dropdown menu items to
conform to Bootstrap 5 requirements. The patch also revises many
instances of Bootstrap tooltips which were not working -- tooltips which
should be shown with many of the disable dropdown items.

In BS5, a tooltip can't be attached to a disabled link. Instead the
tooltip must be attached to the link's container. These changes also
include the addition of "aria-disabled='true'" to the disabled menu
items.

Sponsored-by: Athens County Public Libraries
Signed-off-by: David Nind <david@davidnind.com>
Comment 3 David Nind 2024-08-28 02:07:41 UTC
Testing notes (using KTD):

1. For a staff patron (such as Henry), update their permissions so that they can access the staff interface, but not set permissions for other patrons.
2. Log in as that staff patron, and for a patron (for example, Mary) got to More > Set permissions.
3. Note that there is no differentiation with the other menu items, and that there is a hover message "You are not authorized to set permissions".
4. Another place to check is Acquisitions > Administration > Budgets. For the main budget actions menu, Delete and + Add fund.
5. After aplying the patch and rebuilding the CSS (yarn build) notice that the "Set permission" option (and other non-active options) are greyed out.

Note about tooltips:
1. I did note that the tool tip message is now larger in the patron area - more spacing around the text, which is larger (and more readable, for me anyway). 
2. In the Budget/funds area, the tooltip is smaller (text size and less spacing around the text).
Comment 4 Jonathan Druart 2024-08-28 07:20:28 UTC
It's not working on "Update child to adult patron". It is missing `$('[data-bs-toggle="tooltip"]').tooltip()`

We will need to add it for the whole members module. Don't you think we should actually have it directly in staff-global to affect all templates?
Comment 5 Owen Leonard 2024-08-28 11:30:04 UTC Comment hidden (obsolete)
Comment 6 Owen Leonard 2024-08-28 11:32:05 UTC
(In reply to Jonathan Druart from comment #4)
> It's not working on "Update child to adult patron".

Thanks for testing Jonathan. After all that work hoping it would get the "update child" selenium test working I forgot to test the "update child" link X(

> should actually have it directly in staff-global to affect all templates?

The follow-up patch does so.
Comment 7 Jonathan Druart 2024-08-28 11:59:28 UTC
Created attachment 170807 [details] [review]
Bug 37748: Adjust the 'Update child to patron' selenium test
Comment 8 Jonathan Druart 2024-08-28 12:44:52 UTC
1. should we remove this? koha-tmpl/intranet-tmpl/prog/js/messaging-preference-form.js:    $('#memberentry_messaging_prefs [data-bs-toggle="tooltip"][disabled]').tooltip();


2. In koha-tmpl/intranet-tmpl/prog/js/catalog.js

121     $('[data-bs-toggle="tooltip"]').on("click",function(e){
122         e.stopPropagation();
123     }).tooltip();
Shouldn't we have 'disabled' instead of this on click?

3. same in koha-tmpl/intranet-tmpl/prog/en/modules/clubs/clubs.tt

4. This one certainly needs to be removed: koha-tmpl/intranet-tmpl/prog/en/modules/catalogue/itemsearch.tt:                $('[data-bs-toggle="tooltip"]').tooltip();

=> From Owen "The item search one is necessary because the tooltips have to be initialized after the table is drawn and the buttons updated"

5. Is this still relevant? koha-tmpl/intranet-tmpl/prog/js/members-menu.js:    $("#updatechild, #patronflags, #renewpatron, #deletepatron, #exportbarcodes").tooltip();
Comment 9 Owen Leonard 2024-08-28 13:33:32 UTC
Created attachment 170818 [details] [review]
Bug 37748: (follow-up) Move tooltip initialization to staff-global.js

This patch moves generic initializations of Bootstrap tooltips into
staff-global.js.

Templates which included custom tooltip initialization have been updated
to use the global function.
Comment 10 Owen Leonard 2024-08-28 13:36:26 UTC
(In reply to Jonathan Druart from comment #8)
> 2. In koha-tmpl/intranet-tmpl/prog/js/catalog.js
> 
> 121     $('[data-bs-toggle="tooltip"]').on("click",function(e){
> 122         e.stopPropagation();
> 123     }).tooltip();
> Shouldn't we have 'disabled' instead of this on click?

This code was in a couple places but not everywhere... the idea being that if you try to click a disabled menu item the menu doesn't automatically close. This gives the user a chance to look again and see the tooltip explaining why the option is disabled.

I removed the two unusually cases but we ought to talk about whether that is the desirable behavior for all disabled menu items.
Comment 11 Owen Leonard 2024-08-28 13:42:56 UTC
Created attachment 170819 [details] [review]
Bug 37748: In Bootstrap 5 "disabled" class must be on anchor tag, not list item

This patch revises the markup for disabled dropdown menu items to
conform to Bootstrap 5 requirements. The patch also revises many
instances of Bootstrap tooltips which were not working -- tooltips which
should be shown with many of the disable dropdown items.

In BS5, a tooltip can't be attached to a disabled link. Instead the
tooltip must be attached to the link's container. These changes also
include the addition of "aria-disabled='true'" to the disabled menu
items.

Sponsored-by: Athens County Public Libraries
Signed-off-by: David Nind <david@davidnind.com>
Comment 12 Owen Leonard 2024-08-28 13:42:58 UTC
Created attachment 170820 [details] [review]
Bug 37748: Adjust the 'Update child to patron' selenium test

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 13 Owen Leonard 2024-08-28 13:43:00 UTC
Created attachment 170821 [details] [review]
Bug 37748: (follow-up) Move tooltip initialization to staff-global.js

This patch moves generic initializations of Bootstrap tooltips into
staff-global.js.

Templates which included custom tooltip initialization have been updated
to use the global function.
Comment 14 Jonathan Druart 2024-08-29 07:55:26 UTC
Owen, can you confirm you went through all the points of comment 8?
Comment 15 Owen Leonard 2024-08-29 10:21:11 UTC
(In reply to Jonathan Druart from comment #14)
> Owen, can you confirm you went through all the points of comment 8?

Yes:

(In reply to Jonathan Druart from comment #8)
> 1. should we remove this?

Removed.
 
> 2. In koha-tmpl/intranet-tmpl/prog/js/catalog.js
> 3. same in koha-tmpl/intranet-tmpl/prog/en/modules/clubs/clubs.tt

Removed. See comment 10.
 
> 5. Is this still relevant? koha-tmpl/intranet-tmpl/prog/js/members-menu.js: 
> $("#updatechild, #patronflags, #renewpatron, #deletepatron,
> #exportbarcodes").tooltip();

Removed.
Comment 16 Jonathan Druart 2024-08-29 10:49:15 UTC
Created attachment 170860 [details] [review]
Bug 37748: In Bootstrap 5 "disabled" class must be on anchor tag, not list item

This patch revises the markup for disabled dropdown menu items to
conform to Bootstrap 5 requirements. The patch also revises many
instances of Bootstrap tooltips which were not working -- tooltips which
should be shown with many of the disable dropdown items.

In BS5, a tooltip can't be attached to a disabled link. Instead the
tooltip must be attached to the link's container. These changes also
include the addition of "aria-disabled='true'" to the disabled menu
items.

Sponsored-by: Athens County Public Libraries
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 17 Jonathan Druart 2024-08-29 10:49:18 UTC
Created attachment 170861 [details] [review]
Bug 37748: Adjust the 'Update child to patron' selenium test

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 18 Jonathan Druart 2024-08-29 10:49:21 UTC
Created attachment 170862 [details] [review]
Bug 37748: (follow-up) Move tooltip initialization to staff-global.js

This patch moves generic initializations of Bootstrap tooltips into
staff-global.js.

Templates which included custom tooltip initialization have been updated
to use the global function.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 19 Katrin Fischer 2024-08-29 15:25:28 UTC
Pushed for 24.11!

Well done everyone, thank you!