fix(pagination): center the first/last double-chevron caret icons under RTL - #4752
Closed
freddymeta wants to merge 1 commit into
Closed
fix(pagination): center the first/last double-chevron caret icons under RTL#4752freddymeta wants to merge 1 commit into
freddymeta wants to merge 1 commit into
Conversation
…er RTL The prev/next carets were fixed in #4723 by moving the RTL mirror from a display:contents wrapper span onto the Icon via xstyle, so the glyph stays a centered flex child. The input variants first/last (<< / >>) double-chevron buttons still used the old wrapper span, so they kept the same few-pixels-high offset. Apply the identical xstyle fix to both double-chevron carets. Extends the #4723 regression test to assert the mirror class rides on the icon element itself for all four carets (first/prev/next/last).
freddymeta
requested review from
cixzhang,
ejhammond and
imdreamrunner
as code owners
August 6, 2026 12:52
meta-cla
Bot
added
the
CLA Signed
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
community
github-actions
Bot
requested review from
cvkxx,
ernestt,
kentonquatman and
rubyycheung
August 6, 2026 12:52
Contributor
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR Modified ComponentsPagination (@astryxdesign/core) · View in Storybook
Bundle Size Summary
Accessibility AuditStatus: No accessibility violations detected. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
cixzhang
approved these changes
Aug 6, 2026
github-actions
Bot
removed
the
needs:code-review
This was referenced Aug 6, 2026
cixzhang
added a commit
that referenced
this pull request
Aug 7, 2026
) * refactor(core): move RTL mirror styles from icon wrappers onto Icon xstyle The <span {...stylex.props(rtlStyles.mirror)}> wrappers around directional chevrons exist only to carry the mirror transform. Icon extends BaseProps and merges xstyle into the same span it already renders, so the style lands on the same element and one DOM node goes away. Pagination's prev/next carets already use this shape (#4752); this brings first/last, Carousel, Lightbox and the Table row-expansion menu icon in line. Sites: Carousel x2, Lightbox x2, Pagination x2, Table/rowExpansion x1. * refactor(core): move style-only wrapper styles onto the component root Each wrapper here carried only positioning/spacing for the single Astryx component inside it, so the styles move onto that component's own root: - Lightbox close/prev/next: absolute placement onto IconButton's xstyle. navButton switches transform: translateY(-50%) to the individual translate property, because on the Button root a transform replaces the Button's own transform rules and kills its scale(0.98) press feedback; translate composes with them, which is what the wrapper+button pair did before. - Pagination page-size Selector: the wrapper set width, so it becomes the documented width prop. Selector's xstyle targets the trigger box, not the field root, so xstyle would have sized the wrong element. - MobileNav header Heading, Switch FieldStatus, TopNav drawer Divider, TopNavMegaMenu Grid: straight move onto xstyle. Measured with Playwright against Storybook: rendered geometry is unchanged except where the wrapper was actively mis-centering the icon — Lightbox prev/next chevrons and Pagination first/last chevrons were 2.5-3px off the button's vertical center (same defect #4752 fixed for prev/next) and are now centered. * chore(changeset): style-only wrapper migration * docs(utils): rtlStyles now applies to the icons themselves, not wrappers
rubyycheung
approved these changes
Aug 7, 2026
Contributor
Author
|
Closing without merging — superseded by #4775. #4775 ("migrate style-only wrappers onto component xstyle") swept the same change across all four carets: on |
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
3 participants
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.
Summary
Follow-up to #4723. That PR fixed the vertical centering of the prev/next caret icons by moving the RTL mirror off a
display: contentswrapper span and onto theIcondirectly viaxstyle. Theinputvariant's first/last («/») double-chevron buttons were still using the old wrapper span, so they kept the same few-pixels-high offset — the "double caret looks off" bug.This applies the identical, already-reviewed fix to both double-chevron carets, so all four carets (first / prev / next / last) now share one consistent approach.
The fix
Why it slipped through
When #4723 landed, the first/last buttons didn't exist on
mainyet — they came in with theinputvariant (#4505). #4723 could only fix the carets that existed at the time; the double-chevrons inherited the wrapper pattern and shipped with the offset.Tests
Extended the #4723 regression test to cover all four carets in one pass (it now renders the
inputvariant so first/last are present) and to assert the mirror class rides on the icon element itself rather than a wrapper. I verified the test genuinely fails when a caret is reverted to the wrapper form.93/93Pagination tests, typecheck, and strict lint all green.RTL flip is preserved (
scaleX(-1)still applies via the sharedrtlStyles.mirror); centering is verified visually in Storybook.