fix: add app content wrapper padding based on page text direction(ltr or rtl) #1250
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
muhammad-ammar
changed the title
| @@ -141,7 +143,7 @@ class EnterpriseApp extends React.Component { | |||
| tabIndex="-1" | |||
| ref={this.contentWrapperRef} | |||
| style={{ | |||
| paddingLeft: matchesMediaQ ? sidebarWidth : defaultContentPadding, | |||
| [isLocaleRTL ? 'paddingRight' : 'paddingLeft']: matchesMediaQ ? sidebarWidth : defaultContentPadding, | |||
There was a problem hiding this comment.
[suggestion] Rather than relying on getLocale and isRtl here, I would recommend a pure CSS solution with paddingInlineStart here instead, the primary reasons being:
- It's a simpler implementation.
- Can respond to manual changes of the
dir="ltr"attribute on the<html>element, whereas this solution would not. For example, if you manually change the<html>element'sdir="ltr"attribute in DevTools todir="rtl", the padding is not properly updated with the current approach.
The padding-inline-start CSS property (docs) takes into account LTR vs. RTL directionality by default.
Example screenshot, after manually changing to dir="rtl" in DevTools and relying on paddingInlineStart:
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.
Description: Add padding(left or right) baesd on the page text direction(ltr or rtl)
For all changes
Only if submitting a visual change