fix(icon): add :dir fallback #1223
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
liamdebeasi
commented
May 26, 2023
| :host(.flip-rtl:dir(rtl)) .icon-inner { | ||
| transform: scaleX(-1); | ||
| } | ||
| :host(.flip-rtl:dir(ltr)) .icon-inner { |
Contributor
Author
There was a problem hiding this comment.
This is needed for WebKit otherwise the fallback will always cause the icon to be flipped if the document loads in RTL
Contributor
There was a problem hiding this comment.
It would probably be nice to have this comment in the code.
mapsandapps
approved these changes
May 30, 2023
Contributor
mapsandapps
left a comment
mapsandapps
left a comment
There was a problem hiding this comment.
I no longer have the right version of Safari to test this with, but it looks good to me. Thanks for figuring this all out!
| :host(.flip-rtl:dir(rtl)) .icon-inner { | ||
| transform: scaleX(-1); | ||
| } | ||
| :host(.flip-rtl:dir(ltr)) .icon-inner { |
Contributor
There was a problem hiding this comment.
It would probably be nice to have this comment in the code.
sean-perkins
approved these changes
May 30, 2023
Contributor
sean-perkins
left a comment
sean-perkins
left a comment
There was a problem hiding this comment.
Changes look good to me. +1 to adding a comment for why we have scaleX(1) for the ltr direction.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are a couple things we missed with our first pass of #1210:
:host-contextnor:dirhave no fallback to at least set the dir correctly on page load.@supports selector(:dir(rtl))evaluates totruein Safari <16.4 even though:dirwas supported starting in Safari 16.4. So even if we made a fallback such as above, it would not work on older versions of Safari.Solutions:
isRTLcheck so at least the icon respects the dir on component load-webkit-named-imageand applied the RTL fallback. WebKit browsers that support:dir(rtl)will override this fallback and get the reactive RTL behavior.Safari Bug Example: https://codepen.io/liamdebeasi/pen/eYPXPZY
Browsers that do not support
:dir(rtl)should render a red square. Browsers that do support:dir(rtl)should render a green square only when.squarehasdir="rtl"(which it does in this template.Chrome: Red (Chrome doesn't support :dir yet so this result is fine)
Firefox: Green
Safari 16.4: Green
Safari <16.4: Nothing
Safari <16.4 reports that it supports
:dir(rtl)so it never renders the red fallback. However, it never renders the green color either since it does not actually support:dir(rtl).