-
Notifications
You must be signed in to change notification settings - Fork 525
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
Add context to provide rtl support for react-icons #560
base: main
Are you sure you want to change the base?
Conversation
…cons into react-icons-rtl
@@ -16,7 +16,7 @@ | |||
"cleanSvg": "rm -rf ./intermediate", | |||
"copy": "node ../../importer/generate.js --source=../../assets --dest=./intermediate --extension=svg --target=react", | |||
"copy:font-files": "cpy './src/utils/fonts/*.{ttf,woff,woff2,json}' ./lib/utils/fonts/. && cpy './src/utils/fonts/*.{ttf,woff,woff2,json}' ./lib-cjs/utils/fonts/.", | |||
"convert:svg": "node convert.js --source=./intermediate --dest=./src", | |||
"convert:svg": "node convert.js --source=./intermediate --dest=./src --filter=../../importer/rtl.txt", |
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.
I don't see rtl.txt
included in this PR? If there is an existing file, it might be used for a different purpose? We probably want to create a new file for this feature.
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.
The rtl.txt
file is here.
I am wary about creating a new file for this because this file was already created by the designers to track icons that should be flipped in rtl
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.
We need to track down with the Android folks (that's the only platform previously using the rtl.txt file) what is/isn't there. RTL wasn't kept up with, for the most part, as other platforms didn't need/use it.
We should also collaborate with the Apple folks on that, as they are the platform that has the most RTL/LTR support in, and it's supported by the platform itself, but we need to make sure everything has equivalency.
@@ -10,6 +10,7 @@ const _ = require("lodash"); | |||
|
|||
const SRC_PATH = argv.source; | |||
const DEST_PATH = argv.dest; | |||
const RTL_FILTER_PATH =argv.filter; |
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.
filter
is a confusing name for this arg. How about flip_in_rtl
?
Ditto for the file name: flip_in_rtl.txt
instead of rtl.txt
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.
Possibly even autoflip_in_rtl
This PR is to add a react context to the icon build that will allow icons to flip in rtl. This is the first step of the process. The next step is to create/update a list of flippable icons to apply the rtl styling to. Part of #553