We should use the aria-disabled attribute correctly on the breadcrumbs menu. This aids in accessibility, as it signifies to screen readers whether or not a link should be clickable
Created attachment 189033 [details] [review] Bug 41177: Add aria-disabled to breadcrumbs This patch adds aria-disabled and tabindex = -1 to a breadcrumb, if it is the current page. This aids in accesssibility, and is generally good practice. == TO TEST: == a) Visit any page on the OPAC, and inspect the breadcrumb pertinent to the current page *) Notice the lack of aria-disabled and tabindex on the inspected element == APPLY PATCH == b) repeat step a *) Notice how there is now an aria-disabled and tabindex = -1 *) Notice you cannot tab on to the final breadcrumb anymore c) Ensure other breadcrumbs are unaffected and still work as links == SIGN OFF ==
Created attachment 189188 [details] [review] Bug 41177: Add aria-disabled to breadcrumbs This patch adds aria-disabled and tabindex = -1 to a breadcrumb, if it is the current page. This aids in accesssibility, and is generally good practice. == TO TEST: == a) Visit any page on the OPAC, and inspect the breadcrumb pertinent to the current page *) Notice the lack of aria-disabled and tabindex on the inspected element == APPLY PATCH == b) repeat step a *) Notice how there is now an aria-disabled and tabindex = -1 *) Notice you cannot tab on to the final breadcrumb anymore c) Ensure other breadcrumbs are unaffected and still work as links == SIGN OFF == Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 189189 [details] [review] Bug 41177: (follow-up) A few minor corrections This patch makes the following corrections: - Remove link from breadcrumb WRAPPER items. The right markup is generated in html_helpers.inc if you add the "bc_active = 1" to the breadcrumb_item WRAPPER. - Remove "href='#'" from inactive links, and adjust CSS accordingly. The W3C validator doesn't like it when a true "aria-disabled" attribute is on a link which has an href attribute.
Jake I hope my follow-up makes sense!
Created attachment 189261 [details] [review] Bug 41177: (follow-up) A few minor corrections This patch makes the following corrections: - Remove link from breadcrumb WRAPPER items. The right markup is generated in html_helpers.inc if you add the "bc_active = 1" to the breadcrumb_item WRAPPER. - Remove "href='#'" from inactive links, and adjust CSS accordingly. The W3C validator doesn't like it when a true "aria-disabled" attribute is on a link which has an href attribute. Signed-of-by: Jake Deery <jake.deery@openfifth.co.uk>
Created attachment 189262 [details] [review] Bug 41177: (follow-up) A few minor corrections This patch makes the following corrections: - Remove link from breadcrumb WRAPPER items. The right markup is generated in html_helpers.inc if you add the "bc_active = 1" to the breadcrumb_item WRAPPER. - Remove "href='#'" from inactive links, and adjust CSS accordingly. The W3C validator doesn't like it when a true "aria-disabled" attribute is on a link which has an href attribute. Signed-off-by: Jake Deery <jake.deery@openfifth.co.uk>
Thanks for that, Owen. Looks good to me, signed off!
Created attachment 189651 [details] [review] Bug 41177: (follow-up) A few minor corrections This patch makes the following corrections: - Remove link from breadcrumb WRAPPER items. The right markup is generated in html_helpers.inc if you add the "bc_active = 1" to the breadcrumb_item WRAPPER. - Remove "href='#'" from inactive links, and adjust CSS accordingly. The W3C validator doesn't like it when a true "aria-disabled" attribute is on a link which has an href attribute. Signed-off-by: Jake Deery <jake.deery@openfifth.co.uk> Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Created attachment 189652 [details] [review] Bug 41177: Add aria-disabled to breadcrumbs This patch adds aria-disabled and tabindex = -1 to a breadcrumb, if it is the current page. This aids in accesssibility, and is generally good practice. == TO TEST: == a) Visit any page on the OPAC, and inspect the breadcrumb pertinent to the current page *) Notice the lack of aria-disabled and tabindex on the inspected element == APPLY PATCH == b) repeat step a *) Notice how there is now an aria-disabled and tabindex = -1 *) Notice you cannot tab on to the final breadcrumb anymore c) Ensure other breadcrumbs are unaffected and still work as links == SIGN OFF == Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Created attachment 189653 [details] [review] Bug 41177: (follow-up) A few minor corrections This patch makes the following corrections: - Remove link from breadcrumb WRAPPER items. The right markup is generated in html_helpers.inc if you add the "bc_active = 1" to the breadcrumb_item WRAPPER. - Remove "href='#'" from inactive links, and adjust CSS accordingly. The W3C validator doesn't like it when a true "aria-disabled" attribute is on a link which has an href attribute. Signed-off-by: Jake Deery <jake.deery@openfifth.co.uk> Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Nice work everyone! Pushed to main for 25.11
Hi there, I think the second commit breaks Tools/ManageMarcImport_spec.ts, see Koha_Main, run #3445 for reference. Probably just some markup that shifted around.
I don't know Cypress but is this the problematic line? cy.contains("Manage staged MARC records").click(); Is the "click" not being registered because there is not longer an href attribute?
I think the exact error was something like that, yeah. CSS evaluating to `pointer-events: none`.
Created attachment 189931 [details] [review] Bug 41177: Adjust Cypress tests
Created attachment 189932 [details] [review] Bug 41177: Adjust Cypress tests Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
follow-up pushed to main
Nice work everyone! Pushed to 25.05.x
Hard to apply on 24.11.x LTS I skip