refactor(theme): replace the use of RTL-specific styles with logical properties #3868
Conversation
🦋 Changeset detectedLatest commit: 5c09a36 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 |
|
@macci001 is attempting to deploy a commit to the NextUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in this pull request introduce a patch for the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![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
🧹 Outside diff range and nitpick comments (7)
.changeset/giant-worms-hammer.md (1)
1-5: LGTM! Consider enhancing the description.The changeset file is correctly formatted and provides a clear, concise explanation of the patch update to the "@nextui-org/theme" package. The change aligns well with the PR objectives of removing RTL usage.
Consider expanding the description to highlight the benefits of using logical properties, such as improved maintainability and better support for different writing modes. For example:
- The PR replaces the use of RTL-specific styles with logical properties. + The PR replaces the use of RTL-specific styles with logical properties. This change improves maintainability and provides better support for different writing modes, enhancing the overall flexibility of the theme.packages/core/theme/src/components/modal.ts (1)
Line range hint
1-224: Overall excellent implementation with room for minor improvementThe modal component is well-structured and offers great flexibility through its various slots and variants. The use of Tailwind Variants (tv) allows for easy customization and maintainability.
For consistency with the RTL improvement made, consider reviewing other directional properties in the file (if any) to ensure they also use logical properties where applicable. For example, properties like
left,right,margin-left,margin-right, etc., could be replaced with their logical counterparts (inset-inline-start,inset-inline-end,margin-inline-start,margin-inline-end, etc.) for better RTL support.packages/core/theme/src/components/toggle.ts (3)
103-103: Good use of logical properties in size variantsThe changes in the
smsize variant successfully replace RTL-specific classes with logical properties, simplifying the code and maintaining the same layout behavior. This aligns well with the PR's objective.For consistency, consider using
inline-endinstead ofendin theendContentclass (line 58), as it more accurately represents the intended behavior in different writing modes.Also applies to: 107-107
125-125: Consistent use of logical properties in lg size variantThe changes in the
lgsize variant are consistent with thesmandmdvariants, successfully replacing RTL-specific classes with logical properties. This simplification aligns well with the PR's objective while maintaining the intended layout behavior.Consider using CSS variables for the margin and offset values (e.g.,
ms-[--toggle-offset-large]) to allow for easier customization and maintain consistency across size variants.Also applies to: 129-129
Line range hint
172-191: Consider updating compound variants for consistencyWhile the main size variants have been updated to use logical properties, the compound variants still use directional properties like
ml-3,ml-4, andml-5. For consistency with the rest of the changes and to fully support different writing modes, consider updating these to use logical properties as well.For example:
- thumb: ["group-data-[pressed=true]:w-5", "group-data-[selected]:group-data-[pressed]:ml-3"], + thumb: ["group-data-[pressed=true]:w-5", "group-data-[selected]:group-data-[pressed]:ms-3"],packages/core/theme/src/components/checkbox.ts (2)
101-101: Completed RTL support across all size variantsThe change from
mr-2tome-2in the large size variant completes the set of modifications across all size variants of the checkbox component. This consistent approach ensures that the component now fully supports both LTR and RTL layouts, regardless of the chosen size.Consider updating the component's documentation to highlight the improved RTL support, as this change enhances the component's versatility and ease of use in multilingual applications.
Line range hint
81-101: Summary: Successful removal of RTL-specific styling in checkbox componentThe changes made to the checkbox component successfully remove RTL-specific styling by replacing
mr-2withme-2across all size variants (small, medium, and large). This modification aligns perfectly with the PR objective of "Removing the usages of RTL wherever possible."Key points:
- Improved flexibility: The component now adapts to both LTR and RTL layouts automatically.
- Consistent implementation: The change is applied uniformly across all size variants.
- No breaking changes: The component's core functionality and structure remain unchanged.
These updates enhance the component's versatility and ease of use in multilingual applications, adhering to modern CSS best practices for handling different text directions.
To fully leverage these improvements:
- Update the component's documentation to highlight the enhanced RTL support.
- Consider applying similar changes to other components in the library for consistency.
- If not already in place, add RTL layout tests to ensure the changes work as expected in different scenarios.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (8)
- .changeset/giant-worms-hammer.md (1 hunks)
- packages/core/theme/src/components/checkbox.ts (3 hunks)
- packages/core/theme/src/components/date-input.ts (1 hunks)
- packages/core/theme/src/components/input.ts (3 hunks)
- packages/core/theme/src/components/modal.ts (1 hunks)
- packages/core/theme/src/components/radio.ts (1 hunks)
- packages/core/theme/src/components/select.ts (3 hunks)
- packages/core/theme/src/components/toggle.ts (2 hunks)
🧰 Additional context used
🔇 Additional comments (19)
packages/core/theme/src/components/modal.ts (1)
62-62: Excellent use of logical property for RTL supportThe change from
right-1andrtl:left-1toend-1is a great improvement. This logical property automatically adapts to the text direction, simplifying the styling and improving maintainability. It aligns perfectly with the PR objective of removing RTL-specific styles.packages/core/theme/src/components/toggle.ts (3)
57-58: Excellent use of logical properties!The changes to
startContentandendContentclasses effectively replace RTL-specific styles with logical properties. This approach simplifies the code and provides better support for different writing modes, aligning perfectly with the PR's objective.
114-114: Consistent use of logical properties in md size variantThe changes in the
mdsize variant are consistent with thesmvariant, successfully replacing RTL-specific classes with logical properties. This simplification aligns well with the PR's objective while maintaining the intended layout behavior.Also applies to: 118-118
Line range hint
1-199: Overall excellent improvements in RTL handlingThe changes in this file successfully remove RTL-specific classes and replace them with logical properties, aligning perfectly with the PR's objective. This approach simplifies the code, improves maintainability, and provides better support for different writing modes.
The modifications are consistent across different size variants, demonstrating a thorough implementation of the new approach. The suggested minor improvements (using
inline-end, CSS variables for offsets, and updating compound variants) would further enhance the consistency and flexibility of the component.These changes contribute significantly to the overall goal of simplifying the styling approach in the project.
packages/core/theme/src/components/radio.ts (3)
100-100: Excellent use of logical properties!The change from
ml-2 rtl:mr-2 rtl:ml-[unset]toms-2is a great improvement. By using the logical propertyms-2(margin-inline-start), the styling automatically adapts to the text direction, eliminating the need for separate RTL classes. This simplifies the code, improves maintainability, and aligns perfectly with the PR's objective of removing RTL usage where possible.
107-107: Consistent application of logical propertiesThe change from
ml-2 rtl:mr-2 rtl:ml-[unset]toms-2for thelgsize is consistent with the previous change for themdsize. This consistency ensures that the same logical property approach is applied across different sizes of the radio component, further simplifying the styling and improving overall maintainability.
100-107: Summary: Successful removal of RTL-specific classesThe changes made to the
radiocomponent successfully achieve the PR's objective of removing RTL usage where possible. By replacing RTL-specific classes with logical properties (ms-2), the code becomes more maintainable and adaptable to different text directions. These modifications:
- Simplify the styling approach
- Maintain consistent behavior across different writing modes
- Do not introduce any breaking changes
- Are consistently applied across different sizes (
mdandlg)Overall, these changes represent a positive step towards more efficient and flexible styling in the component.
packages/core/theme/src/components/checkbox.ts (2)
81-81: Improved RTL support with logical propertyThe change from
mr-2tome-2enhances the checkbox component's adaptability to different text directions. This logical property automatically adjusts the margin based on the writing mode, supporting both LTR and RTL layouts without additional configuration.
91-91: Consistent RTL support in medium size variantThe replacement of
mr-2withme-2in the medium size variant maintains consistency with the small size variant. This change ensures that the checkbox component behaves consistently across different sizes while supporting both LTR and RTL layouts.packages/core/theme/src/components/date-input.ts (2)
169-169: Excellent use of logical properties!The change from
left-0 rtl:right-0tostart-0is a great improvement. This logical property automatically adjusts based on the text direction, eliminating the need for explicit RTL handling. This simplifies the code, improves maintainability, and aligns perfectly with the PR's objective of removing RTL usage where possible.
175-175: Consistent application of logical propertiesThis change mirrors the previous one, replacing
left-0 rtl:right-0withstart-0for theoutside-leftplacement. This consistency is commendable, ensuring a uniform approach across different placement options. It further reinforces the move towards logical properties and away from explicit RTL handling.packages/core/theme/src/components/select.ts (4)
13-13: Improved writing mode supportThe change from "origin-top-left" to "origin-top-start" enhances the component's adaptability to different writing modes. This logical property approach eliminates the need for explicit RTL classes, aligning with the PR's objective of removing RTL usage where possible.
26-26: Enhanced text alignment for multi-directional supportChanging "text-left" to "text-start" improves the component's flexibility in handling different text directions. This logical property approach ensures correct text alignment regardless of the writing mode, supporting the PR's goal of simplifying RTL handling.
190-190: Consistent use of logical properties for animationsThe modification from "origin-top-left" to "origin-top-start" in the
disableAnimationvariant ensures consistency with earlier changes. This approach maintains the component's adaptability to different writing modes even when animations are disabled, furthering the PR's objective of simplifying RTL support.
Line range hint
1-624: Summary: Successful implementation of logical properties for improved RTL supportThe changes in this file consistently replace directional properties with logical ones, specifically:
- "origin-top-left" to "origin-top-start"
- "text-left" to "text-start"
These modifications align perfectly with the PR's objective of removing explicit RTL usage. By utilizing logical properties, the select component now adapts more seamlessly to different writing modes without the need for separate RTL styles. This approach simplifies the codebase and improves maintainability while ensuring consistent behavior across various language directions.
packages/core/theme/src/components/input.ts (4)
31-31: Improved label origin with logical propertyThe change from "origin-top-left rtl:origin-top-right" to "origin-top-start" is a good improvement. It simplifies the code by using a logical property that automatically adapts to the text direction, making the component more maintainable and consistent across different writing systems.
191-191: Simplified padding with logical property in isClearable variantThe change from "peer pr-6 rtl:pr-0 rtl:pl-6" to "peer pe-6" is an excellent improvement. It uses the logical property "pe" (padding-end) which automatically adapts to the text direction. This simplification makes the code more maintainable and reduces the need for separate RTL styles.
210-210: Enhanced isRequired variant with logical propertyThe modification from "after:content-[''] after:text-danger after:ml-0.5 rtl:after:ml-[unset] rtl:after:mr-0.5" to "after:content-[''] after:text-danger after:ms-0.5" is a significant improvement. By using the logical property "ms" (margin-start), the code now automatically adapts to different text directions. This change simplifies the styling, improves maintainability, and eliminates the need for separate RTL adjustments.
Line range hint
1-724: Overall improvement in input component stylingThe changes made to this file consistently replace RTL-specific styles with logical properties throughout the input component. This approach aligns perfectly with the PR objectives of removing RTL usage where possible. The use of logical properties like
origin-top-start,pe-6, andms-0.5instead of direction-specific classes simplifies the codebase, improves maintainability, and ensures better adaptability across different writing systems.These modifications will make the input component more flexible and easier to maintain in the long run, reducing the need for separate RTL styles and potential inconsistencies between LTR and RTL layouts.
wingkwong
requested review from
wingkwong
and removed request for
jrgarciadev
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
wingkwong
changed the title
Closes #
📝 Description
This PR removes the usages of the RTL in styles wherever possible.
Refer: link
⛳️ Current behavior (updates)
🚀 New behavior
💣 Is this a breaking change (Yes/No): No
Summary by CodeRabbit