Skip to content

ui-kit(pagination): PaginationEllipsis's "More pages" sr-only label sits inside an aria-hidden subtree, so it is never announced #10052

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

PaginationEllipsis puts aria-hidden on its outer wrapper and a screen-reader-only label inside it:

// packages/loopover-ui-kit/src/components/pagination.tsx:97-109
const PaginationEllipsis = ({
  className,
  ...props
}: React.ComponentProps<"span">) => (
  <span
    aria-hidden
    className={cn("flex h-9 w-9 items-center justify-center", className)}
    {...props}
  >
    <MoreHorizontal className="h-4 w-4" />
    <span className="sr-only">More pages</span>
  </span>
);

aria-hidden="true" removes the element and its entire subtree from the accessibility tree. The nested
sr-only span is inside that subtree, so "More pages" is removed along with the icon: it is announced by
nothing, and the gap in the page-number sequence is presented to assistive tech as an unlabelled hole. The
sr-only class only hides text visually — it is not an escape hatch from an ancestor's aria-hidden.

The two pieces contradict each other: either the whole control is decorative (in which case the sr-only
label is dead markup that will keep being copied into new components) or the label is meant to be heard (in
which case aria-hidden on the wrapper is wrong). Today it is written both ways and behaves as the first.

This package already has the correct pattern for exactly this shape. TypingIndicator scopes aria-hidden to
the decorative elements only and keeps its label outside them:

// packages/loopover-ui-kit/src/components/typing-indicator.tsx:21-25
      <span className="sr-only">{label}</span>
      <span
        aria-hidden="true"
        className="size-1.5 animate-bounce rounded-full bg-muted-foreground ..."
      />

packages/loopover-ui-kit/src/components/pagination.test.tsx covers PaginationLink's aria-disabled
styling (#8307) and never renders PaginationEllipsis, so nothing pins this.

Requirements

  • aria-hidden must be moved off the outer <span> and applied to the decorative <MoreHorizontal /> icon
    only, so the sr-only "More pages" text remains in the accessibility tree and is announced.
  • The rendered DOM must otherwise be unchanged: the same outer <span> element, the same
    flex h-9 w-9 items-center justify-center classes merged through cn with a caller's className, the same
    {...props} spread, the same MoreHorizontal icon with h-4 w-4, and the same sr-only child text
    "More pages".
  • A caller must still be able to override the behaviour: an explicitly passed aria-hidden prop must win,
    because {...props} is spread after the component's own attributes. Do not reorder the spread.
  • PaginationEllipsis.displayName must stay "PaginationEllipsis".
  • No other export in packages/loopover-ui-kit/src/components/pagination.tsx may change — in particular
    PaginationLink's aria-current and its aria-disabled:pointer-events-none aria-disabled:opacity-50
    classes are load-bearing (PaginationLink's aria-disabled has no visual/pointer-events effect, unlike sidebar.tsx's and calendar.tsx's aria-disabled: styling #8307/fix(ui-kit): give PaginationLink aria-disabled a real visual/interaction effect #8359) and must be left exactly as they are.

⚠️ Required pattern: mirror packages/loopover-ui-kit/src/components/typing-indicator.tsx:21-25 — label
outside, aria-hidden on the decorative nodes. What does NOT satisfy this issue: (a) deleting the sr-only
span and leaving aria-hidden on the wrapper, which removes the only description of the gap in the page
sequence rather than fixing it; (b) replacing the sr-only child with an aria-label on an aria-hidden
wrapper (an aria-hidden element contributes no accessible name either); (c) adding a role to the outer
span; (d) a repo-wide aria-hidden audit across every ui-kit component — this issue is
PaginationEllipsis only; (e) a test-only PR.

Deliverables

  • packages/loopover-ui-kit/src/components/pagination.tsx's PaginationEllipsis renders its outer
    <span> without aria-hidden, and the MoreHorizontal icon with aria-hidden.
  • Rendering <PaginationEllipsis /> and querying by text "More pages" finds an element that has no
    aria-hidden="true" ancestor, asserted in packages/loopover-ui-kit/src/components/pagination.test.tsx.
  • Rendering <PaginationEllipsis className="mx-2" /> produces an outer span whose className contains
    both mx-2 and h-9, asserted in the same file (the cn merge is unchanged).
  • Rendering <PaginationEllipsis aria-hidden /> still produces an outer span with aria-hidden="true",
    asserted in the same file (a caller override still wins).
  • The existing PaginationLink / PaginationPrevious / PaginationNext tests in that file still pass
    unmodified.
  • A regression test at packages/loopover-ui-kit/src/components/pagination.test.tsx named for this bug.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one
that removes aria-hidden from the wrapper but asserts only getByText("More pages") (which the buggy markup
also satisfies, since the text is in the DOM either way) — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include
covers src/**/*.ts, packages/loopover-engine/src/**/*.ts, packages/loopover-{miner,mcp}/{lib,bin}/**/*.ts,
packages/loopover-contract/src/**/*.ts and packages/discovery-index/src/**/*.ts. It does not cover
packages/loopover-ui-kit/**, and that package's own packages/loopover-ui-kit/vitest.config.ts deliberately
declares no coverage block. Codecov does not gate the patch on this change.

The tests are still mandatory and are named above: they go in the existing
packages/loopover-ui-kit/src/components/pagination.test.tsx and must be run with
npm --workspace @loopover/ui-kit run test before pushing (that suite is not currently reached by
npm run test:ci, so a local run is the only way to see it pass).

PaginationEllipsis has no conditional of its own, so the branch requirement lands on the cn merge and the
props spread: assert the default render (no caller className, no caller aria-hidden) and the overridden
render (both supplied) — the two arms of {...props} winning or not applying — plus the presence-of-label
assertion above, so neither the merge nor the override can silently regress.

Expected Outcome

A screen-reader user reaches the gap in a paginated sequence and hears "More pages" instead of nothing, and
the component stops shipping an sr-only label that structurally cannot be read — removing the contradiction
between its markup and its intent.

Links & Resources

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions