Skip to content

Proof of concept for rtlcss alternative to cssjanus#92

Merged
ItsJonQ merged 2 commits intomasterfrom
try/rtlcss
Nov 18, 2020
Merged

Proof of concept for rtlcss alternative to cssjanus#92
ItsJonQ merged 2 commits intomasterfrom
try/rtlcss

Conversation

@sarayourfriend
Copy link
Copy Markdown
Collaborator

@sarayourfriend sarayourfriend commented Nov 6, 2020 •
edited
Loading

Testing instructions

Boot up storybook and find the html element. Add a dir="rtl" attribute to it and verify that RTL support is working as expected (compare against using cssjanus to check that everything looks the same.

@vercel
Copy link
Copy Markdown

vercel bot commented Nov 6, 2020 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/itsjonq/g2/7q7pe92l0
✅ Preview: https://g2-git-try-rtlcss.itsjonq.vercel.app

sarayourfriend
sarayourfriend commented Nov 6, 2020
View reviewed changes
@sarayourfriend sarayourfriend mentioned this pull request Nov 6, 2020
Closed
@sarayourfriend sarayourfriend self-assigned this Nov 6, 2020
@ItsJonQ
Copy link
Copy Markdown
Owner

ItsJonQ commented Nov 6, 2020

Screen Shot 2020-11-06 at 1 03 44 PM

Holy smokes! It works 🎉 !!!

@saramarcondes I'm 💯 down with this solution. If you're okay with it, feel free to tidy this PR up and we'll get it merged 🙌

@ItsJonQ
Copy link
Copy Markdown
Owner

ItsJonQ commented Nov 6, 2020

Oh dang. I just noticed the browser version of rtlcss.js uses the full postcss library.

Which is about 20kb gzipped 😱
https://bundlephobia.com/result?p=postcss@8.1.6

That feels really heavy 😅

CSSJanus comes in at 1.7kb gzipped:
https://bundlephobia.com/result?p=cssjanus@2.0.1

@ItsJonQ ItsJonQ linked an issue Nov 16, 2020 that may be closed by this pull request
Styles: RTL Plugin: Figure out a GPL v2.0 compatible solution #90
Closed
@ItsJonQ
Copy link
Copy Markdown
Owner

ItsJonQ commented Nov 17, 2020 •
edited
Loading

From what I gather... the only concern with this approach would be the bundle size (which is a valid concern).

I poked around, and it doesn't look like rtlcss or postcss introduces any side-effects (e.g. doing strange CSS things in the global space). Thank goodness for that.

It looks like this may be our only option for now (besides re-writing CSSJanus).
The bundle size is unfortunate, but it may be trade off we have to make.

Because this RTL plugin is a tiny module of create-style, the RTL parser can be something we can (very) easily swap out in the future.

With that said, I'm okay moving forward with this approach.

@saramarcondes What do you think :)

@sarayourfriend
Copy link
Copy Markdown
Collaborator Author

sarayourfriend commented Nov 17, 2020

@ItsJonQ Sounds good to me!

@sarayourfriend sarayourfriend marked this pull request as ready for review November 17, 2020 21:56
ItsJonQ
ItsJonQ approved these changes Nov 18, 2020
View reviewed changes
Copy link
Copy Markdown
Owner

@ItsJonQ ItsJonQ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 from me!!! Thank you @saramarcondes 🙏

@ItsJonQ ItsJonQ merged commit 6c135d0 into master Nov 18, 2020
@ItsJonQ ItsJonQ added this to the Prep: Gutenberg Integration milestone Nov 18, 2020
@ItsJonQ ItsJonQ deleted the try/rtlcss branch December 10, 2020 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ItsJonQ ItsJonQ ItsJonQ approved these changes

Assignees

@sarayourfriend sarayourfriend

Labels

styles systems

Projects

None yet

Milestone

Phase 1: Prep

Development

Successfully merging this pull request may close these issues.

Styles: RTL Plugin: Figure out a GPL v2.0 compatible solution

2 participants

@sarayourfriend @ItsJonQ