Description
Owen Leonard
2024-08-27 11:19:34 UTC
Created attachment 170784 [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 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> 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). 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? Created attachment 170804 [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. (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. Created attachment 170807 [details] [review] Bug 37748: Adjust the 'Update child to patron' selenium test 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(); 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. (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. 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> Created attachment 170820 [details] [review] Bug 37748: Adjust the 'Update child to patron' selenium test Signed-off-by: Owen Leonard <oleonard@myacpl.org> 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. Owen, can you confirm you went through all the points of comment 8? (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. 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> 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> 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> Pushed for 24.11! Well done everyone, thank you! |