Repository navigation
fix: keep screen-reader-only text inside its component - #54
Open
mdarikrayhan wants to merge 1 commit into
Open
mdarikrayhan wants to merge 1 commit into
mdarikrayhan wants to merge 1 commit into
Conversation
sr-only text is absolutely positioned. When none of its ancestors inside
the component is positioned, it is placed against the nearest positioned
ancestor outside it. In an app whose content scrolls inside a container
(a full-height shell with a scrolling main that isn't positioned), that
is the page itself. The text then sits at its unscrolled position, far
below the window, so the document grows taller than the window, and
scrolling past the end of the content moves the whole layout up and
shows a blank strip below it. The FlyCommerce dashboard hit this on its
Orders list with the DataTable loading status added in 0.3.0.
A gallery audit (each sr-only element's containing block, compared with
the component it belongs to) found five such places, and each now has a
positioned parent:
- DataTable: the card holding the loading status;
- DataTable: the "More pages" label of the page numbers;
- PaginationEllipsis;
- CopyButton's "Copied" live region;
- SidebarTrigger's "Toggle Sidebar" label.
The table-header labels ("Actions", "Reorder") already resolve to the
table's own positioned container, so they are unchanged. A positioned
parent with no offsets doesn't move anything.
Deploying flycommerce-ui with
|
| Latest commit: |
481ebd9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f3d42b60.flycommerce-ui.pages.dev |
| Branch Preview URL: | https://fix-data-table-contain-sr-on.flycommerce-ui.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem.
sr-onlytext is absolutely positioned. Several components had no positioned ancestor of their own around it, so it was placed against the nearest positioned ancestor outside the component.In an app whose content scrolls inside a container (a full-height shell with a scrolling
<main>that isn't positioned), that ancestor is the page. The text sits at its unscrolled position, far below the window, which makes the document taller than the window. Scrolling past the end of the content then moves the whole layout up and leaves a blank strip below it.The FlyCommerce dashboard hit this on its Orders list after upgrading to 0.3.1.
DataTable's loading status, added in 0.3.0 (#40), sat 1,375px down in a 900px window, and the window scrolled 476px.Change. A positioned parent (
relative, no offsets, so nothing moves) for each place a gallery audit found:DataTable: the card that holds the loading status region;DataTable: the "More pages" label in the numbered pagination;PaginationEllipsis;CopyButton: the "Copied" live region (classNameis now merged rather than spread over);SidebarTrigger: the "Toggle Sidebar" label.Audit. For every absolutely positioned
sr-onlyelement in the gallery, I compared its containing block with the fc component it belongs to (its closest[data-slot]ancestor).sidebar-insetor header.Checked locally on Node 22 with the CI steps: typecheck, lint, test (183 tests, every gallery demo rendered with axe), build, check:package, build:site, and
skill:refswith a cleanplugin/diff.🤖 Generated with Claude Code