Skip to content
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

Fix RTL margin handling in group component #25

Merged
merged 2 commits into from
Jan 3, 2025
Merged

Conversation

remohammadi
Copy link

I had made a mistake previously by putting the RTL margin styles one level higher than where it belonged.

remohammadi and others added 2 commits January 2, 2025 01:42
I had made a mistake previously by putting the RTL margin styles one level higher than where it belonged.
@Yohn
Copy link
Owner

Yohn commented Jan 3, 2025

Looks good! Thanks for the RTL updates as well, its much appreciated and helps a lot!

Do you work with RTL styling often? I haven't done any RTL styling before, and kind of understand it, but it would be great to have someone that understands it better than I do, to ensure compatibility with those languages.

@Yohn Yohn merged commit db14c89 into Yohn:main Jan 3, 2025
@remohammadi
Copy link
Author

Thank you for the feedback and for maintaining this project. I appreciate your kind words 😃

I've worked on RTL styling in the past, although it's not a regular part of my current projects. I'm happy to help ensure compatibility with RTL languages as much as I can.

Please let me know if there's anything specific I can assist with.

@Yohn
Copy link
Owner

Yohn commented Jan 8, 2025

Hey @remohammadi
Thanks again for the help!
One RTL thing I've noticed more of is the use of padding and margin -left/right/top/bottom instead of the newer inline-start, inline-end, block-start and block-end..

I grabbed a pull request from the original PicoCSS project picocss/pull/599 and made a pull request here for it #27, but I know it would need to be updated as well for some newer features that been added. Would you mind possibly doing a find/replace within the scss directory to update that to use the logical properties for better RTL support? You can make a new pull request if you're able to do that for us! If not its cool, I'll get to it probably next week after I finish this project for a client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants