refactor(core): mirror Pagination chevrons under RTL via CSS, not JS direction read - #4687
Draft
nynexman4464 wants to merge 5 commits into
Draft
refactor(core): mirror Pagination chevrons under RTL via CSS, not JS direction read#4687nynexman4464 wants to merge 5 commits into
nynexman4464 wants to merge 5 commits into
Conversation
…direction read Pagination was the last component selecting a physical icon name from a render-time useDirection() read. Switch it to render fixed chevronLeft/ chevronRight glyphs wrapped in the shared rtlStyles.mirror (scaleX(-1)), so they flip purely from an ancestor's dir via the CSS cascade -- matching Calendar and the rest of the library. This removes the only internal consumer of the i18n direction context and eliminates a potential server/client hydration mismatch on the chevrons. No API change.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
requested review from
cvkxx,
ernestt,
kentonquatman and
rubyycheung
August 3, 2026 19:49
Consumer side: add an 'it just works' note (set dir once, components mirror themselves) and correct the stale caveat that claimed overlays (menus/dialogs) aren't mirrored yet -- they mirror via logical CSS anchor positioning now (#4568 and the Layer logical placement). Contributor side: replace the single 'read useDirection() to swap an icon' example -- the exact anti-pattern this stack removes from Pagination -- with a priority hierarchy: (1) logical CSS, (2) icons via rtlStyles.mirror, (3) behavioral logic via lazy isRtlElement / focus-hook auto-detect, (4) useDirection() context only as a render-time last resort. Notes that isRtl on the focus hooks is deprecated and that no component reads direction from context at render time after the CSS-mirror migration.
The docsite prose renderer (renderInlineMarkdown) only supports backtick code-spans and [label](href) links -- **bold** renders as literal asterisks. Convert the four contributor priority tiers from fake-bold prose lead-ins into real level-4 headings + prose, which the renderer supports and which gives proper visual hierarchy. Code identifiers stay in the prose (where backticks work), since headings render text verbatim.
nynexman4464
force-pushed
the
nynexman4464/refactor/pagination-css-mirror
branch
from
August 3, 2026 21:54
4411afc to
9996c02
Compare
The hook's own doc-comment still told authors to reach for it to 'swap directional icons' -- the exact pattern the Pagination refactor and the updated guide move away from. Reframe it to point at the preferred tools first (logical CSS, rtlStyles.mirror for icons, lazy isRtlElement for behavior) and note the hydration caveat, so IDE hovers and the API surface match the guidance. Comment-only.
The provider's dir prop feeds context direction only; it never touches the DOM dir attribute. Since astryx components mirror layout and directional icons from the DOM dir (not the context value), an RTL locale won't visually mirror unless the consumer also sets dir on <html> or a wrapping element. Make this explicit where a provider user will look: add the previously-undocumented dir prop to the component doc, a best-practice entry, and expand the dir JSDoc (IDE hover). Docs only.
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
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.
Why
Pagination was the last component in the library that chose a physical icon name from a render-time
useDirection()read to flip its prev/next chevrons under RTL. Everything else (Calendar, Carousel, Table disclosure, SideNav, …) mirrors directional icons purely with CSS via the sharedrtlStyles.mirror(scaleX(-1)), flipping from an ancestor'sdirthrough the cascade.Two payoffs:
directionfrom context at render time — every internal RTL decision resolves from the DOM (logical CSS + CSS mirror + lazy DOM reads for behavior).useDirection()remains a public, SSR-safe convenience for consumers who need render-time direction in their own client components.directionand the actual<html dir>could disagree and flash the wrong chevron on hydration. Pure CSS has no such branch.What
Code: Render fixed
chevronLeft/chevronRightglyphs, each wrapped in<span {...stylex.props(rtlStyles.mirror)}>, and drop theuseDirection()-based name-swap.aria-labels are unchanged (they were already semantic, not directional). No API change; patch-level.Docs (
internationalization.doc.mjs) — brought in line with reality now that no component reads direction at render time:dironce, components mirror themselves" note, and corrected a stale caveat that claimed pop-up overlays (menus/dialogs) aren't mirrored yet. They mirror now via logical CSS anchor positioning (Layer logical placement + Dialogstart/end, feat(core): add logical start/end to Dialog position; deprecate physical left/right #4568).useDirection()to swap an icon" example — the exact anti-pattern this PR removes — with a priority hierarchy: (1) logical CSS, (2) directional icons viartlStyles.mirror, (3) behavioral logic via lazyisRtlElement/focus-hook auto-detect, (4)useDirection()context only as a render-time last resort. Also notesisRtlis deprecated on the focus hooks.Testing
@astryxdesign/coretypecheck clean, ESLint clean,check:synccleandirtoggle)