pagy icon indicating copy to clipboard operation
pagy copied to clipboard

Bug: Issue with screenreader because rel attribute on prev/next removed (incorrectly established "they don't do anything")

Open SleeplessByte opened this issue 8 months ago • 8 comments

👀 Before submitting...

  • [x] I upgraded to pagy version 9.3.4
  • [x] I searched through the Documentation
  • [x] I searched through the Issues
  • [x] I searched through the Q&A
  • [x] I asked Pagy Trained AI - Warning - The information may not be 100% accurate, but it serves as a good starting point for further investigation.

🧐 REQUIREMENTS

  • [x] I am providing a VALID code file that confirms the bug
  • [x] I am NOT posting any of the USELESS THINGS listed above
  • [x] I am aware that this issue will be automatically closed if the code file is missing or INVALID

💬 Description

  1. Reproduction: repro.ru.txt (no changes made)
  2. What actually happens: When using any screenreader (NVDA + Chrome used) the previous and next link are announced according to the aria-label attached, but are not recognised as "next" and "previous" links, breaking any override for the next/previous links, breaking shortcuts set up to navigate, and potentially breaking voice commands depending on the used assistive technology.
  3. What should happen: The prev and next links should have rel="prev" and rel="next", when enabled, so that screenreaders, shortcuts, and voice commands work as intended.

Relevant code change was introduced here: https://github.com/ddnexus/pagy/commit/48382d6fd22ccda0b4e52aa1aca0d2de8ef56a27#diff-06572a96a58dc510037d5efa622f9bec8519bc1beab13c9f251e97e657a9d4edR54

Workaround is to not use pagy_nav and self-implement, which we have done now.

SleeplessByte avatar Apr 10 '25 17:04 SleeplessByte

Yeah, I thought readers were less dumb, and able to derive the rel info by the context, but indeed they totally rely on explicit external info to work properly.

AI made me too optimistic 😀.

Will fix in the upcoming v10

ddnexus avatar Apr 11 '25 04:04 ddnexus

You are the best! Thank you.

SleeplessByte avatar Apr 13 '25 16:04 SleeplessByte

Hey. thanks for your work.

Another common scenario is to target a[rel='next'] for infinite scrolling implementation.

olimart avatar May 07 '25 19:05 olimart

@olimart I don't get how you would get any attribute from the UI in that case 🤔

ddnexus avatar May 08 '25 00:05 ddnexus

I guess @olimart means using JavaScript.

e.g.:

let lastLink = undefined

// run this in a scroll observer or something similar
const nextLink = document.querySelector('.pagy a[rel="next"][href]:not([href=""]):enabled')
if (nextLink && lastLink != nextLink.getAttribute('href')) {
  const focusBefore = document.activeElement

  // Optionally hook into loading the contents once to announce it to assistive tech
  const onPageLoaded = () => { /* ... */ }
  document.addEventListener('ajax:succes', onPageLoaded, { once: true })

  // Click next link
  nextLink.click();
  lastLink = nextLink.getAttribute('href');

  // Restore focus in case any handling moved the focus
  if (focusBefore) {
    focusBefore.focus();
  }
}

SleeplessByte avatar May 08 '25 12:05 SleeplessByte

indeed, I meant targeting the next link with JS, similar to const next = this.paginationTarget.querySelector("[rel=next]") to decide if we need to fetch more records

olimart avatar May 08 '25 12:05 olimart

Infinite scrolling exists exactly to avoid using a pagination UI. Using JavaScript to target the "next" link of a pagination nav, doesn't make any sense to me.

And if you don't target a full pagination nav, but just a hidden link to the next page, then you don't even need the rel selector, but just an attribute id.

I still don't see any "common scenario" doing that, and if you really do that, and that is "common", then I will add a big warning in the docs, explaining a few better alternatives to avoid it.

ddnexus avatar May 08 '25 15:05 ddnexus

(For this issue I only care about the screenreader I use 😀)

SleeplessByte avatar May 08 '25 16:05 SleeplessByte

Fixed in 43.0.0

ddnexus avatar Aug 10 '25 07:08 ddnexus