-
Notifications
You must be signed in to change notification settings - Fork 587
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(components): improve RTL support #2433
Conversation
switch
in RTL mode
src/theme/input.ts
Outdated
@@ -17,40 +17,40 @@ export default (options: Required<ModuleOptions>) => ({ | |||
size: { | |||
xs: { | |||
base: 'px-2 py-1 text-xs gap-1', | |||
leading: 'pl-2', | |||
trailing: 'pr-2', | |||
leading: '!ps-2', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why add !
important class here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Screen.Recording.2024-10-22.at.2.16.53.PM.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing the class in the chrome devtools won't work, the ps-9
class doesn't exist since it isn't defined anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
screen-recording-2024-10-23-at-92255-am_npHC5fsW.mp4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in version 4.0.0-alpha.28, the ps-10
is not applied as expected because px-5
takes precedence, while in version 3.4.13, ps-10
works correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an issue with Tailwind then, can't merge this with important values it will prevent override.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This issue has been solved : tailwindlabs/tailwindcss#14772
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Let's wait for the new release then :) Thanks for taking care of the issue on tailwindcss!
Are you planning on improving other components or only the Input and Switch had issues? |
Yes,I have a plan to complete |
@Malik-Jouda What about it? |
Suggest adding an example of how to implement multi-directional support. |
commit: |
@Malik-Jouda I've merged |
@Malik-Jouda Is this good to merge or should I wait? |
Yes β |
@Malik-Jouda Sorry I introduced a conflict in my latest commit, can you have a look? π¬ |
π Linked issue
β Type of change
π Description
π Checklist