Description
Wainui Witika-Park
2021-04-07 03:08:18 UTC
Created attachment 119933 [details] [review] Bug 28101: changed styling for breadcrumbs Updates opac.scss file so the breadcrumbs will appear the same as Bug 27846 (Staff Client breadcrumbs). This works but the file may need to be changed/cleaned up so that there is not unnecessary information in it. Sponsored-by: Catalyst IT Created attachment 119934 [details] [review] Bug 28101: changed breadcrumbs in the OPAC Modified breadcrumbs to be accessible, in particular for a screen-reader. Also ensured the breadcrumbs were all consistent. Made the block of breadcrumbs to be a <nav id="breadcrumbs" aria-label="Breadcrumb" class="breadcrumb"> with an ordered list inside. The last breadcrumb also has aria-current="page" to specify that it is the current page. To test: 1) Apply patch 2) Build scss file 3) Ensure each of the files have breadcrumbs that are in a <nav id="breadcrumbs" aria-label="Breadcrumb" class="breadcrumb"> block 4) Ensure that there is an ordered list in the block of breadcrumbs 5) Ensure that the last breadcrumb has aria-current="page" 6) Ensure that the breadcrumbs on each page of the OPAC looks basically the same as before, but the '>' symbol is more bold, and the last breadcrumb has bold text 7) Ensure that when the last breadcrumb is clicked it takes you to the page you are currently on Sponsored-by: Catalyst IT Please refer to Bug 27846 for discussion around the styling of breadcrumbs I'm curious about your choice to remove the Bootstrap classes from the markup: - <li class="breadcrumb-item" aria-current="page"> + <li> + <a href="#" aria-current="page"> Blocked record + </a> </li> I see that the "aria-current" information has moved to the link, and that's fine with me--I see that it fits the guidelines better. But I think there is value to keeping the Bootstrap 4 markup classes for consistency's sake and to be able to inherit existing Bootstrap classes. (In reply to Owen Leonard from comment #4) > I'm curious about your choice to remove the Bootstrap classes from the > markup: > > - <li class="breadcrumb-item" aria-current="page"> > + <li> > + <a href="#" aria-current="page"> > Blocked record > + </a> > </li> > > I see that the "aria-current" information has moved to the link, and that's > fine with me--I see that it fits the guidelines better. But I think there is > value to keeping the Bootstrap 4 markup classes for consistency's sake and > to be able to inherit existing Bootstrap classes. Hi Owen, thanks for your comment. I was just attempting make it the same as Bug 27846. Please feel free to make any amendments that you believe would work better but that also keep in line with accessibility standards. Created attachment 125184 [details] [review] Bug 28101: changed styling for breadcrumbs Updates opac.scss file so the breadcrumbs will appear the same as Bug 27846 (Staff Client breadcrumbs). This works but the file may need to be changed/cleaned up so that there is not unnecessary information in it. Sponsored-by: Catalyst IT Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 125185 [details] [review] Bug 28101: Update breadcrumb markup in the OPAC for consistency and accessibility Modified breadcrumbs to be accessible, in particular for a screen-reader. Also ensured the breadcrumbs were all consistent. Made the block of breadcrumbs to be a <nav id="breadcrumbs" aria-label="Breadcrumb" class="breadcrumbs"> with an ordered list inside. The last breadcrumb also has aria-current="page" to specify that it is the current page. To test, apply the patch and rebuild the OPAC CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) - Confirm that OPAC templates are updated consistently to use breadcrumbs markup beginning with '<nav id="breadcrumbs" aria-label="Breadcrumb" class="breadcrumb">' - Pages in the OPAC should look consistent, with the last breadcrumb styled as text and with the "aria-current" attribute "page." Sponsored-by: Catalyst IT Created attachment 126028 [details] [review] Bug 28101: Update breadcrumb markup in the OPAC for consistency and accessibility Modified breadcrumbs to be accessible, in particular for a screen-reader. Also ensured the breadcrumbs were all consistent. Made the block of breadcrumbs to be a <nav id="breadcrumbs" aria-label="Breadcrumb" class="breadcrumbs"> with an ordered list inside. The last breadcrumb also has aria-current="page" to specify that it is the current page. To test, apply the patch and rebuild the OPAC CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) - Confirm that OPAC templates are updated consistently to use breadcrumbs markup beginning with '<nav id="breadcrumbs" aria-label="Breadcrumb" class="breadcrumb">' - Pages in the OPAC should look consistent, with the last breadcrumb styled as text and with the "aria-current" attribute "page." Sponsored-by: Catalyst IT Signed-off-by: Henry Bolshaw <bolshawh@parliament.uk> Created attachment 126804 [details] [review] Bug 28101: changed styling for breadcrumbs Updates opac.scss file so the breadcrumbs will appear the same as Bug 27846 (Staff Client breadcrumbs). This works but the file may need to be changed/cleaned up so that there is not unnecessary information in it. Sponsored-by: Catalyst IT Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 126805 [details] [review] Bug 28101: Update breadcrumb markup in the OPAC for consistency and accessibility Modified breadcrumbs to be accessible, in particular for a screen-reader. Also ensured the breadcrumbs were all consistent. Made the block of breadcrumbs to be a <nav id="breadcrumbs" aria-label="Breadcrumb" class="breadcrumbs"> with an ordered list inside. The last breadcrumb also has aria-current="page" to specify that it is the current page. To test, apply the patch and rebuild the OPAC CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) - Confirm that OPAC templates are updated consistently to use breadcrumbs markup beginning with '<nav id="breadcrumbs" aria-label="Breadcrumb" class="breadcrumb">' - Pages in the OPAC should look consistent, with the last breadcrumb styled as text and with the "aria-current" attribute "page." Sponsored-by: Catalyst IT Signed-off-by: Henry Bolshaw <bolshawh@parliament.uk> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> In the first patch there is a li::before with a svg image background (that is the ">" I think). However I am not seeing it, it's still "/", not ">" like on the staff interface. Am I reading wrongly those changes? Created attachment 126833 [details] [review] Bug 28101: (follow-up) Correct breadcrumb divider image This patch corrects the CSS for generating a chevron-style divider between breadcrumb items. The CSS copied from staff-global.scss needed some tweaks to make it work in the OPAC with Bootstrap 4. Also changed: The Base64-encoded SVG is converted to plain based on https://css-tricks.com/probably-dont-base64-svg/ Created attachment 126834 [details] [review] Bug 28101: (follow-up) Correct breadcrumb divider image This patch corrects the CSS for generating a chevron-style divider between breadcrumb items. The CSS copied from staff-global.scss needed some tweaks to make it work in the OPAC with Bootstrap 4. Also changed: The Base64-encoded SVG is converted to plain based on https://css-tricks.com/probably-dont-base64-svg/ Nitpicking, why height: 0.8em? This is 0.8em: https://snipboard.io/gIsDzV.jpg Vs 0.6em: https://snipboard.io/PGHrtz.jpg The chevron is more vertically aligned with the text. This is for a follow-up bug anyway. Thanks for the follow-up, Owen! Pushed to master for 21.11, thanks to everybody involved! Nitpicking, why height: 0.8em? This is 0.8em: https://snipboard.io/r2anVS.jpg Vs 0.6em: https://snipboard.io/PGHrtz.jpg The chevron is more vertically aligned with the text. This is for a follow-up bug anyway. Thanks for the follow-up, Owen! |