feat(ui): RTL support #7718
feat(ui): RTL support #7718
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 698c014 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| <html class="h-full"> | ||
| <html | ||
| class="h-full" | ||
| dir="rtl" |
There was a problem hiding this comment.
For demo purposes, will revert before merging.
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR converts physical directional CSS properties to logical equivalents across the UI (e.g., left/right → insetInlineStart/insetInlineEnd; marginLeft/marginRight → marginInlineStart/marginInlineEnd; paddingLeft/paddingRight → paddingInlineStart/paddingInlineEnd; border radii → logical radii) and changes textAlign from left/right to start/end. It adds dir="rtl" to the sandbox HTML template, updates several icon/arrow transforms for RTL, and introduces an ESLint rule to enforce logical CSS properties. All changes are styling-related; component logic and public APIs remain unchanged. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. No actionable comments were generated in the recent review. 🎉 Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/ui/src/elements/Avatar.tsx (2)
156-175:⚠️ Potential issue | 🟠 MajorReplace physical
leftwith logicalinsetInlineStartto keep RTL behavior correct.Anchoring the shimmer to the physical left breaks RTL alignment; it should start at the inline-start edge. Please switch to logical inset properties for both the wrapper and its
:afterto preserve RTL support.Proposed fix
position: 'absolute', top: 0, - left: 0, + insetInlineStart: 0, width: '25%', height: '100%', @@ position: 'absolute', top: 0, - left: 0, + insetInlineStart: 0, width: '400%', height: '100%',
25-37:⚠️ Potential issue | 🟠 MajorAdd/confirm RTL regression coverage for Avatar shimmer.
No tests were added/updated alongside this RTL-facing change. Please add or confirm existing visual/snapshot coverage for RTL positioning to prevent regressions. As per coding guidelines: If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
🧹 Nitpick comments (1)
packages/ui/src/elements/TagInput.tsx (1)
112-176: Add minimal RTL regression coverage for these styling changes.No tests were added/updated in this PR; please add a basic RTL-focused snapshot/visual test to prevent layout regressions. As per coding guidelines: If there are no tests added or modified as part of the PR, please suggest that tests be added to cover the changes.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/elements/Header.tsx (1)
1-113:⚠️ Potential issue | 🟠 MajorAdd RTL-focused tests before merge.
This PR changes RTL-facing UI behavior but adds no tests. Please add/adjust tests (e.g., RTL snapshot/visual regression for BackLink and arrow icons) to prevent regressions.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/ui/src/elements/Badge.tsx`:
- Around line 28-30: The badge X translate is not RTL-aware: in Badge.tsx update
the transform logic that currently uses translate(${t.space.$2x5}, ...) (paired
with insetInlineEnd) to handle RTL like Button.tsx does — wrap the transform in
a '[dir="rtl"] &' rule and provide the mirrored X value (or use scaleX(-1)) so
the horizontal offset is negated for RTL; keep the insetInlineEnd change and
ensure the selector targets the same class/element that currently has transform
applied.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ui/src/components/UserProfile/DeleteSection.tsx (1)
19-49:⚠️ Potential issue | 🟠 MajorAdd/confirm RTL coverage tests before merge.
This PR changes RTL styling but no tests are shown in the diff. Please add/adjust RTL visual or unit coverage (or point to existing tests) to validate these UI changes.
🤖 Fix all issues with AI agents
In `@packages/ui/src/components/UserProfile/PasswordSection.tsx`:
- Around line 39-43: Add an RTL-focused React Testing Library test for the
PasswordSection component to cover the paddingInlineStart behavior change:
render <PasswordSection /> (or the parent wrapper that sets passwordEnabled)
with dir="rtl" and with both passwordEnabled true and false, then assert the
computed style or DOM placement (e.g., check container.firstChild or the
specific element that receives the sx prop) for paddingInlineStart being '0'
when passwordEnabled is false and undefined/absent when true; use render from
`@testing-library/react` (and getComputedStyle or toHaveStyle assertions) and name
the test clearly (e.g., "renders correct paddingInlineStart in RTL") so this
regression is prevented.
2 participants
Description
Preview: https://clerk-js-sandbox-git-alexcarpenter-rtl-support.clerkstage.dev/sign-in
Inspired by https://x.com/shadcn/status/2017287507881164816 update positioning values to use logical properties to better support RTL usage.
This also adds an eslint rule to prevent
noPhysicalCssPropertiesusage. Note some cases need to still use physical css properties and those have been disable with reasonings why.fixes #3961
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit