feat: add force left/right rules to RTL preset#1179
Conversation
|
Deployment failed with the following error: |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThe pull request introduces force-* direction utilities to the RTL preset that bypass RTL logic. It adds new test cases for force-* tokens and corresponding CSS utility definitions. The changes include new handler functions for forced padding, margin, positioning, text alignment, rounded corners and border widths. Updated warning messages guide users to either use suggested RTL alternatives or the force-* options to retain original physical directions. The CSS snapshots reflect the new default-layer block containing explicit force-* utility definitions. Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
uno-preset-rtl.ts (1)
22-28:⚠️ Potential issue | 🟠 MajorFix force- guidance for variant-prefixed utilities.*
force-${match}yields invalid ordering for variants (e.g.,sm:pl-2→force-sm:pl-2). The force prefix needs to be applied to the utility segment after the last variant prefix (e.g.,sm:force-pl-2). This currently misleads users and will need snapshot updates once fixed.💡 Suggested fix
function reportWarning(match: string, suggestedClass: string, checker?: CollectorChecker) { - const message = `${checker ? 'a' : 'A'}void using '${match}', use '${suggestedClass}' instead, or 'force-${match}' to keep physical direction.` + const forceClass = match.replace(/([^:]+)$/, 'force-$1') + const message = `${checker ? 'a' : 'A'}void using '${match}', use '${suggestedClass}' instead, or '${forceClass}' to keep physical direction.` if (checker) { checker(message, match) } else { warnOnce(`[RTL] ${message}`, match) } }
| /^force-(?:position-|pos-)?(left|right)-(.+)$/, | ||
| ([_, direction, size], context) => { | ||
| // Map 'left'/'right' to 'l'/'r' for directionMap lookup if needed, | ||
| // but directionMap has 'left'/'right' keys? No, it has 'l'/'r'. | ||
| // Wait, directionMap keys are 'l', 'r'. | ||
| // But inset usually uses 'left', 'right' properties directly. | ||
| // Let's use a custom handler for inset to be safe. | ||
| const v = | ||
| (context.theme as unknown as any).spacing?.[size || 'DEFAULT'] ?? | ||
| h.bracket?.cssvar?.global?.auto?.fraction?.rem?.(size!) | ||
| if (v != null) { | ||
| return [[direction === 'left' ? 'left' : 'right', v]] | ||
| } | ||
| }, |
There was a problem hiding this comment.
Remove the any cast to keep this rule type-safe.
context.theme as unknown as any breaks strict type-safety. You can destructure theme using RuleContext<any> (like the helper does) and avoid the cast.
💡 Suggested fix
- ([_, direction, size], context) => {
+ ([_, direction, size], { theme }: RuleContext<any>) => {
// Map 'left'/'right' to 'l'/'r' for directionMap lookup if needed,
// but directionMap has 'left'/'right' keys? No, it has 'l'/'r'.
// Wait, directionMap keys are 'l', 'r'.
// But inset usually uses 'left', 'right' properties directly.
// Let's use a custom handler for inset to be safe.
const v =
- (context.theme as unknown as any).spacing?.[size || 'DEFAULT'] ??
+ theme.spacing?.[size || 'DEFAULT'] ??
h.bracket?.cssvar?.global?.auto?.fraction?.rem?.(size!)
if (v != null) {
return [[direction === 'left' ? 'left' : 'right', v]]
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /^force-(?:position-|pos-)?(left|right)-(.+)$/, | |
| ([_, direction, size], context) => { | |
| // Map 'left'/'right' to 'l'/'r' for directionMap lookup if needed, | |
| // but directionMap has 'left'/'right' keys? No, it has 'l'/'r'. | |
| // Wait, directionMap keys are 'l', 'r'. | |
| // But inset usually uses 'left', 'right' properties directly. | |
| // Let's use a custom handler for inset to be safe. | |
| const v = | |
| (context.theme as unknown as any).spacing?.[size || 'DEFAULT'] ?? | |
| h.bracket?.cssvar?.global?.auto?.fraction?.rem?.(size!) | |
| if (v != null) { | |
| return [[direction === 'left' ? 'left' : 'right', v]] | |
| } | |
| }, | |
| /^force-(?:position-|pos-)?(left|right)-(.+)$/, | |
| ([_, direction, size], { theme }: RuleContext<any>) => { | |
| // Map 'left'/'right' to 'l'/'r' for directionMap lookup if needed, | |
| // but directionMap has 'left'/'right' keys? No, it has 'l'/'r'. | |
| // Wait, directionMap keys are 'l', 'r'. | |
| // But inset usually uses 'left', 'right' properties directly. | |
| // Let's use a custom handler for inset to be safe. | |
| const v = | |
| theme.spacing?.[size || 'DEFAULT'] ?? | |
| h.bracket?.cssvar?.global?.auto?.fraction?.rem?.(size!) | |
| if (v != null) { | |
| return [[direction === 'left' ? 'left' : 'right', v]] | |
| } | |
| }, |
2 participants
No description provided.