ojs icon indicating copy to clipboard operation
ojs copied to clipboard

pkp/pkp-lib#6296 Article breadcrumbs use aria-current but do not point to the current page - OJS 3.4

Open israelcefrin opened this issue 1 year ago • 5 comments

This PR fixes the issue https://github.com/pkp/pkp-lib/issues/6296 and also implements:

  • [x] aria-current="page" attribute was not being use on a link to the current page
  • [x] Removed aria-current="page" attribute from li element, it should to be used on link elements only
  • [x] Include a link, using the page title as the label, to the last item on the breadcrumb (article and archive pages)
  • [x] Added aria-hidden="true" to the breadcrumb separators (they were announced as "slash" by screen readers which is annoying for screen reader users and don't add any information to the context)

Even though showing the full article title might look overwhelming for sighted people, it is helpfull to people that rely on assistive technology like screen readers or screen magnifiers. They can double-check whether they are on the expected page through the breadcrumb interface element.

israelcefrin avatar Jan 05 '24 04:01 israelcefrin

Hi @asmecher , could you please have a look and let me know if everything is fine? This PR fixes the main issue from this one: https://github.com/pkp/pkp-lib/issues/6296 and also covers more improvements on ARIA attributes that were missing or misplaced.

israelcefrin avatar Jan 05 '24 18:01 israelcefrin

@jardakotesovec, could you review this? I'm probably the last person who should be looking at front-end markup :)

asmecher avatar Jan 09 '24 16:01 asmecher

@israelcefrin Thank you! Looks good to me. You mentioned that the aria-current should be used only on links? If thats the case, should not we also update the other cases to be links? For example if we go to /index.php/publicknowledge/about, than About the Journal is not link.

jardakotesovec avatar Jan 11 '24 13:01 jardakotesovec

@israelcefrin Thank you! Looks good to me. You mentioned that the aria-current should be used only on links? If thats the case, should not we also update the other cases to be links? For example if we go to /index.php/publicknowledge/about, than About the Journal is not link.

Hi @jardakotesovec , you are correct. And I am already tackling it , i.e. the top menu , on this issue: https://github.com/pkp/pkp-lib/issues/5199

israelcefrin avatar Jan 11 '24 15:01 israelcefrin

Hi @jardakotesovec , I've implemented the same feature on the breadcrumbs of OPS and OMP Default Theme breadcrumb template files.

OPS 3.4 PR: https://github.com/pkp/ops/pull/630 OMP 3/4 PR: https://github.com/pkp/pkp-lib/pull/9684

israelcefrin avatar Feb 02 '24 04:02 israelcefrin