feat: add RTL direction infrastructure and migrate primitives to logical properties#320
Draft
garrity-miepub wants to merge 15 commits into
Draft
feat: add RTL direction infrastructure and migrate primitives to logical properties#320garrity-miepub wants to merge 15 commits into
garrity-miepub wants to merge 15 commits into
Conversation
garrity-miepub
commented
Jul 22, 2026
garrity-miepub
commented
Collaborator
- Storybook Direction toolbar (auto/ltr/rtl) wired into applyGlobalTheme; Arabic RTL sample locale
- useDirection() hook + isRtlLocale() for JS-level direction logic
- cn(): physical/logical utilities merge as conflicts so consumer overrides (ml-2) still replace internal logical classes (ms-1); config exported for consumers
- scripts/rtl-scan.mjs ratchet guard (baseline: 540 matches/120 files) wired into CI
- Badge, Input, Label, Text, Switch migrated to logical properties / rtl: variants
- Tailwind safelists updated for new class strings
…cal properties - Storybook Direction toolbar (auto/ltr/rtl) wired into applyGlobalTheme; Arabic RTL sample locale - useDirection() hook + isRtlLocale() for JS-level direction logic - cn(): physical/logical utilities merge as conflicts so consumer overrides (ml-2) still replace internal logical classes (ms-1); config exported for consumers - scripts/rtl-scan.mjs ratchet guard (baseline: 540 matches/120 files) wired into CI - Badge, Input, Label, Text, Switch migrated to logical properties / rtl: variants - Tailwind safelists updated for new class strings
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces right-to-left (RTL) direction support across the component library by adding direction-detection utilities, migrating a set of primitives to Tailwind logical properties / rtl: variants, and enforcing ongoing migration via a CI “ratchet” scan that prevents new physical-direction Tailwind classes from being introduced.
Changes:
- Add
useDirection()+isRtlLocale()helpers and wire Storybook globals (locale + direction) to set the documentdir. - Update
cn()to use an extendedtailwind-mergeconfig so physical (ml-*) and logical (ms-*) utilities conflict and consumer overrides keep working. - Add an RTL scan script + baseline and run it in CI; update Tailwind safelists for new class strings; migrate selected primitives to logical utilities.
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/utils/index.ts | Re-export miewebUITwMergeConflicts as part of the public utils surface. |
| src/utils/cn.ts | Extend tailwind-merge conflicts to treat physical/logical directional utilities as mutually exclusive. |
| src/utils/cn.test.ts | Add unit tests validating cn() conflict behavior for physical ⇄ logical utilities. |
| src/tailwind-preset.ts | Safelist new RTL/logical and rtl: variant class strings for Tailwind preset consumers. |
| src/tailwind-preset.cjs | Mirror safelist updates in the CJS preset build output. |
| src/hooks/useDirection.ts | Add direction hook + locale-based RTL detection helpers. |
| src/hooks/index.ts | Export new direction utilities from the hooks entrypoint. |
| src/components/Text/Text.tsx | Migrate alignment variants from text-left/right to text-start/end. |
| src/components/Switch/Switch.tsx | Add RTL-aware thumb translation via rtl: variants. |
| src/components/Label/Label.tsx | Replace ml-* with logical ms-* for required indicator spacing. |
| src/components/Input/Input.tsx | Replace ml-* with logical ms-* for required indicator spacing. |
| src/components/Badge/Badge.tsx | Replace mr-* with logical me-* for icon spacing. |
| scripts/rtl-scan.mjs | Add ratcheting RTL guard script to detect newly introduced physical-direction Tailwind utilities. |
| scripts/rtl-baseline.json | Add baseline counts per file for the ratchet guard. |
| package.json | Add rtl:scan and rtl:scan:update scripts. |
| .storybook/preview.tsx | Add Storybook direction toolbar and apply global dir based on locale/direction. |
| .gitignore | Ignore a local RTL plan file. |
| .github/workflows/ci.yml | Run RTL guard in CI to prevent regression during migration. |
20 tasks
The variable-length lookbehind was valid JS (ES2018 lookbehinds are not fixed-length restricted, unlike PCRE/Python) and ran green in CI, but a consumed boundary group is more portable and avoids reviewer confusion. Scan output is byte-identical (540 matches / 120 files).
Deploying ui with
|
| Latest commit: |
bf90b2b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://788b5001.ui-6d0.pages.dev |
| Branch Preview URL: | https://feat-rtl-direction-switch.ui-6d0.pages.dev |
Pre-existing gap surfaced by review: bare translate-x-* tokens don't generate the data-[state=checked]: variant for TW3 safelist consumers. Adds the LTR variants alongside the RTL ones.
…dd Text start/end align values - Migrate 13 physical-direction classes in components newly merged from main (MediaEditor, ScriptPanel, SpeedMarkerMenu, TranscriptView) — unblocks the CI RTL guard without adding baseline debt - Text: add first-class align="start"/"end" values; left/right remain as documented direction-aware aliases (review feedback) - Safelist new logical class strings in both presets; tighten baseline (539)
…er behavior 8 tests: RTL locale detection (incl. subtags/underscores/casing), default ltr, pre-mount rtl, ref-scoped resolution with local dir override, and MutationObserver-driven updates on document and nested elements. jsdom lacks the UA dir cascade, so tests set both the dir attribute (observer) and inline direction style (getComputedStyle).
AppHeader, Card, Sheet, Sidebar, Table — 32 physical-direction classes converted (Modal/Tabs/Breadcrumb/Pagination were already clean). - Sheet: side left/right now use start/end positioning; new side="start"/ "end" first-class values with left/right as direction-aware aliases - CardBadge: logical corner positioning; new top-start/top-end/ bottom-start/bottom-end position values, physical names kept as aliases - Sidebar: mobile off-canvas slide gains rtl:translate-x-full so the drawer hides off the correct edge in RTL - Safelist new logical class strings in both presets; tighten baseline 539 -> 507 matches (120 -> 115 files) Verified in Storybook: RTL mirrors (Sheet end-side pins left, Sidebar fully mirrored), LTR unchanged.
…itch # Conflicts: # .gitignore
Refactors merged from main (useAnchoredPosition/useClickOutside adoption) removed physical-direction classes from 8 files.
- scripts/rtl-codemod.mjs: rewrites physical Tailwind utilities to logical equivalents (dry-run by default, --write to apply); preserves variant prefixes and negative signs; flags space-x-*/divide-x-* as NEEDS-MANUAL - Pilot: CookieConsent banner/compact positions -> start/end (10 tokens); verified in Storybook (RTL mirrors, LTR unchanged) - Safelist new tokens in both Tailwind presets; tighten baseline to 478/114 - CardBadge: document position left/right values as direction-aware aliases in public JSDoc (PR review feedback)
- Codemod pass: 72 physical->logical replacements across 16 files (insets, margins/paddings, text alignment, corner radii, borders) - MessageBubble read-receipt avatar stack: add rtl:space-x-reverse - Harden scanner+codemod: restrict inset/spacing suffixes to real Tailwind values (no more prose matches like 'right-click'); exempt the left-1/2 + translate-x-1/2 centering idiom and space-x/divide-x lines already carrying their rtl:*-reverse remedy - Safelist new logical tokens in both presets; baseline 478 -> 387
- border-l-warning -> border-s-warning (MediaEditor emits the logical class on this branch) - Mirror main's media-stack safelist block into the CJS preset to keep both presets in sync
Comment on lines
+69
to
+71
| // Apply text direction (RTL preview) at the document level so CSS logical | ||
| // properties and `rtl:` variants respond everywhere, including docs pages. | ||
| document.documentElement.setAttribute('dir', resolveGlobalDirection(globals)); |
Collaborator
Author
There was a problem hiding this comment.
Good catch — fixed in 4457b26. applyGlobalTheme now sets <html lang> from the same locale global used for direction resolution (defaulting to en), right alongside the dir update, so both the toolbar-change and initial-URL code paths stay in sync. Verified live: ?globals=locale:ar;direction:auto yields lang="ar" + dir="rtl".
Comment on lines
+126
to
+141
| function walk(path, files = []) { | ||
| if (statSync(path).isFile()) { | ||
| files.push(path); | ||
| return files; | ||
| } | ||
| for (const entry of readdirSync(path, { withFileTypes: true })) { | ||
| const p = join(path, entry.name); | ||
| if (entry.isDirectory()) walk(p, files); | ||
| else if ( | ||
| /\.tsx$/.test(entry.name) && | ||
| !/\.(stories|test)\.tsx$/.test(entry.name) | ||
| ) | ||
| files.push(p); | ||
| } | ||
| return files; | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
Agreed — fixed in 4457b26. Extracted an isComponentSource() predicate and applied it to both walked directory entries and explicitly-passed file paths, so --write on a .stories.tsx/.test.tsx/non-TSX file is now a no-op (verified: passing Messaging.stories.tsx directly reports 0 files).
…mod paths - Storybook applyGlobalTheme now sets <html lang> from the locale global alongside dir, so screen readers and locale-sensitive text shaping reflect the selected locale (PR review feedback) - rtl-codemod: explicitly-passed files go through the same .tsx/no-stories/no-tests filter as directory walks (PR review feedback)
Comment on lines
+45
to
+60
| const [direction, setDirection] = useState<Direction>(() => | ||
| resolveDirection(elementRef?.current) | ||
| ); | ||
|
|
||
| useEffect(() => { | ||
| const update = () => setDirection(resolveDirection(elementRef?.current)); | ||
| update(); | ||
| // dir changes are rare; observing the whole tree for just this attribute is cheap. | ||
| const observer = new MutationObserver(update); | ||
| observer.observe(document.documentElement, { | ||
| attributes: true, | ||
| attributeFilter: ['dir'], | ||
| subtree: true, | ||
| }); | ||
| return () => observer.disconnect(); | ||
| }, [elementRef]); |
Collaborator
Author
There was a problem hiding this comment.
Fair point — fixed in bf90b2b. Added a second effect keyed on the resolved ref target (elementRef?.current ?? null), so a re-render that swaps the referenced node re-syncs direction immediately without waiting for a dir mutation. (A dep-less per-render sync was the first attempt, but our react-hooks/exhaustive-deps config rejects setState in an effect without a dep array — keying on node identity is tighter anyway.) Regression test added: ref swap from an LTR node to an RTL node on re-render flips the returned value.
- Prettier pass over the 8 files rewrapped by the codemod (CI format check failure) - useDirection: add an effect keyed on the resolved ref target so a re-render that swaps elementRef.current to a node with a different inherited direction re-syncs without a dir mutation (PR review feedback); regression test added
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.