Skip to content
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.

Sign up for GitHub

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

Fixes #34855 RangeControl style issue in RTL #35777

Merged
merged 3 commits into from
Oct 25, 2021
Merged

Fixes #34855 RangeControl style issue in RTL #35777

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
38ec3f9
Fixes #34855 RangeControl style issue in RTL
pbearne Oct 19, 2021
251101a
addjest the dot offset so it lines up with tool tip
pbearne Oct 20, 2021
e3520fc
swapped to using translateX for the adjustment
pbearne Oct 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/components/src/range-control/styles/range-control-styles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ export const ThumbWrapper = styled.span`
transform: translateX( 4.5px );

${ thumbColor };
${ rtl( { marginLeft: -10 } ) };
${ rtl( { marginLeft: -10 }, { marginRight: 1 } ) };
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure it makes sense that a symmetrical element like this requires asymmetrical margins. In fact the translateX( 4.5px ) a few lines above stands out to me as something that is not being RTL'ed yet. That might also fix the tooltip, which still isn't perfectly aligned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

change the translate didn't fix it
but I was able to tweak the position of the dot so it lines up better
image
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, have we tried something like this yet? The reason I would prefer a symmetric solution if possible is that it's easier to fix when an underlying size value changes. Less guess work and manual tweaking involved.

-	transform: translateX( 4.5px );

	${ thumbColor };
-	${ rtl( { marginLeft: -10 }, { marginRight: 1 } ) };
+	${ rtl( { marginLeft: -10 } ) };
+	${ rtl(
+		{ transform: 'translateX( 4.5px )' },
+		{ transform: 'translateX( -4.5px )' }
+	) };

`;

const thumbFocus = ( { isFocused } ) => {
Expand Down