refactor: replace right/left with inline corresponding values for better RTL support
#560
Conversation
This file contains 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
shuuji3
commented
Jun 9, 2024
| @@ -1108,7 +1103,6 @@ td { | |||
| background-color: var(--pico-background-color); | |||
| color: var(--pico-color); | |||
| font-weight: var(--pico-font-weight); | |||
| text-align: left; | |||
There was a problem hiding this comment.
left and start are identical so they are not needed at the current time when IE doesn't exist.
Comment on lines
-1432
to
-1433
| padding-right: calc(var(--pico-form-element-spacing-horizontal) + 1.5rem) !important; | ||
| padding-left: var(--pico-form-element-spacing-horizontal); |
| background-image: none !important; | ||
| } | ||
| } | ||
| [dir=rtl] :is([type=date], [type=datetime-local], [type=month], [type=time], [type=week]) { | ||
| text-align: right; | ||
| text-align: start; |
There was a problem hiding this comment.
start in [dir=rtl] is the right position.
| padding: calc(var(--pico-block-spacing-vertical) * 0.66) var(--pico-block-spacing-horizontal); | ||
| background-color: var(--pico-card-sectioning-background-color); | ||
| } | ||
| article > header { | ||
| margin-top: calc(var(--pico-block-spacing-vertical) * -1); | ||
| margin-bottom: var(--pico-block-spacing-vertical); | ||
| border-bottom: var(--pico-border-width) solid var(--pico-card-border-color); | ||
| border-top-right-radius: var(--pico-border-radius); | ||
| border-top-left-radius: var(--pico-border-radius); | ||
| border-start-start-radius: var(--pico-border-radius); |
There was a problem hiding this comment.
This might be confusing but border-*-*-radius has also inline/block corresponding.
ref. https://developer.mozilla.org/en-US/docs/Web/CSS/border-end-end-radius
Comment on lines
+667
to
+668
| margin-inline: auto; | ||
| padding-inline: var(--pico-spacing); |
There was a problem hiding this comment.
*-right and *-left can be merged into *-inline.
shuuji3
changed the title
shuuji3
changed the title
right/left with inline corresponding values for better rtl support
shuuji3
changed the title
right/left with inline corresponding values for better rtl supportright/left with inline corresponding values for better RTL support
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.
Hello, I've noticed that there are still dozens of usages of
rightorleftfor positioning, instead ofinline-startorinline-end.This PR replaces those with
inline-*corresponding. Usinginline-*variants will improve support for RTL languages and can simplify CSS properties in some cases.IE didn't support
inline-*variants but IE reached EoL and Pico CSS v2 dropped its support and all modern browsers supportinline-*so now we should be able to useinline-*values everywhere.(Let me know if this PR is too large. I can split it into small pieces if you like.)