Skip to content

Conversation

@rcourtie
Copy link

  • Add react-with-direction to detect RTL/LTR
  • Change how styles are passed to default child components
    e.g. DefaultHandle, DefaultProgressBar so they're properly
    RLT/LTR-ified
  • Change how handle position is calculated based on direction

Also added new storybook examples, some tests, and a note in the docs.

Rodolphe Courtier added 3 commits July 30, 2019 09:43
- Add react-with-directions to detect RTL/LTR
- Change how styles are passed to default child components
  e.g. DefaultHandle, DefaultProgressBar so they're properly
  RLT/LTR-ified
- Change how handle position is calculated based on direction
@rcourtie rcourtie changed the title Rodo add rtl support Add RTL support Jul 30, 2019
Copy link
Collaborator

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

Thanks, this is a great addition!

src/Slider.jsx Outdated
}
return ((x - sliderBox.left) / sliderBox.width) * PERCENT_FULL;
if (direction === DIRECTIONS.RTL) {
return 100 - (((x - sliderBox.left) / sliderBox.width) * PERCENT_FULL);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we pull the ((x - sliderBox.left) / sliderBox.width) * PERCENT_FULL out into its own variable?

Copy link
Collaborator

@majapw majapw left a comment

Choose a reason for hiding this comment

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

Looks great, one small nit!

})
.add('RTL Custom Handle', () => {
function MyHandle({
styles, css, style, handleRef, ...passProps
Copy link
Collaborator

Choose a reason for hiding this comment

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

all these object keys should be in their own line, since the entire literal doesn’t fit on its own line.

@majapw majapw merged commit 9e877ad into airbnb:master Aug 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants