fix(pagination): add aria attributes for screen reader accessibility#47
Conversation
commit: |
|
@msmps small merge conflict in this one FYI |
Docs PreviewCommit: |
6a06ca0 to
18c3966
Compare
|
/bonk please review |
|
@stritt Bonk workflow was cancelled. View workflow run · To retry, trigger Bonk again. |
|
/bigbonk your turn to review |
There was a problem hiding this comment.
Good accessibility improvement overall — wrapping controls in <nav> and adding aria-live for status announcements are the right moves. However, the aria-live / aria-atomic attributes are only applied in the legacy rendering path. The compound component Pagination.Info (the recommended API) doesn't get the same treatment, which means most new consumers won't benefit from this fix.
Specifically, the PaginationInfo component (line ~117-124) renders a plain <div> without aria-live or aria-atomic. Since the compound API is the recommended approach going forward, that's the more important place to add these attributes:
// In PaginationInfo (~line 118)
<div
data-slot="pagination-info"
aria-live="polite"
aria-atomic="true"
className={cn("text-sm text-kumo-strong", className)}
>Also, to answer the question in the PR description: yes, <nav> is preferred over <div role="navigation"> — using semantic HTML is the right call here.
|
Review submitted successfully. Here's a summary of what I flagged: Request Changes on PR #47 with two key points:
I also confirmed that |
stritt
left a comment
There was a problem hiding this comment.
looks good, thanks for addressing the feedback! 👍
73f3ee2 to
735f1b6
Compare



























Description
<nav aria-label="Pagination">aria-live="polite"andaria-atomic="true"to "Showing X-Y of Z" text for page change announcementsnavigationtoPaginationLabelsfor i18n supportProblem
<div>Solution
Add semantic HTML and ARIA attributes:
<nav aria-label={labels.navigation}>(defaults to "Pagination", customizable for i18n)aria-live="polite"- Announces page range changes without interrupting useraria-atomic="true"- Ensures the entire status text is announced as a wholeTesting
Verified with VoiceOver on macOS: