-
Notifications
You must be signed in to change notification settings - Fork 816
Add RTL support for List component #150
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -396,6 +396,7 @@ export default function createListComponent({ | |
| itemStyleCache[index] = style = { | ||
| position: 'absolute', | ||
| left: direction === 'horizontal' ? offset : 0, | ||
| right: direction === 'horizontal' ? offset : 0, | ||
| top: direction === 'vertical' ? offset : 0, | ||
| height: direction === 'vertical' ? size : '100%', | ||
| width: direction === 'horizontal' ? size : '100%', | ||
|
|
@@ -448,7 +449,7 @@ export default function createListComponent({ | |
| } | ||
|
|
||
| _onScrollHorizontal = (event: ScrollEvent): void => { | ||
| const { scrollLeft } = event.currentTarget; | ||
| const { scrollLeft, scrollWidth, clientWidth } = event.currentTarget; | ||
| this.setState(prevState => { | ||
| if (prevState.scrollOffset === scrollLeft) { | ||
| // Scroll position may have been updated by cDM/cDU, | ||
|
|
@@ -457,11 +458,15 @@ export default function createListComponent({ | |
| return null; | ||
| } | ||
|
|
||
| const isRtl = this.props.style && this.props.style.direction === 'rtl'; | ||
|
|
||
| return { | ||
| isScrolling: true, | ||
| scrollDirection: | ||
| prevState.scrollOffset < scrollLeft ? 'forward' : 'backward', | ||
| scrollOffset: scrollLeft, | ||
| scrollOffset: isRtl | ||
| ? scrollWidth - clientWidth - scrollLeft | ||
| : scrollLeft, | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know if this is right. Why should we invert the element's
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry. I think I was mistaken. |
||
| scrollUpdateWasRequested: false, | ||
| }; | ||
| }, this._resetIsScrollingDebounced); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| import { FixedSizeList as List } from 'react-window'; | ||
|
|
||
| const Column = ({ index, style }) => ( | ||
| <div style={style}>Column {index}</div> | ||
| ); | ||
|
|
||
| const Example = () => ( | ||
| <List | ||
| direction="horizontal" | ||
| height={75} | ||
| itemCount={1000} | ||
| itemSize={100} | ||
| width={300} | ||
| style={{ direction: "rtl" }} | ||
| > | ||
| {Column} | ||
| </List> | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import { FixedSizeList as List } from 'react-window'; | ||
|
|
||
| const Row = ({ index, style }) => ( | ||
| <div style={style}>Row {index}</div> | ||
| ); | ||
|
|
||
| const Example = () => ( | ||
| <List | ||
| height={150} | ||
| itemCount={1000} | ||
| itemSize={35} | ||
| width={300} | ||
| style={{ direction: "rtl" }} | ||
| > | ||
| {Row} | ||
| </List> | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| import React, { PureComponent } from 'react'; | ||
| import { FixedSizeList } from 'react-window'; | ||
| import CodeBlock from '../../components/CodeBlock'; | ||
| import ProfiledExample from '../../components/ProfiledExample'; | ||
|
|
||
| import CODE_HORIZONTAL from '../../code/FixedSizeListHorizontalRtl.js'; | ||
| import CODE_VERTICAL from '../../code/FixedSizeListVerticalRtl.js'; | ||
|
|
||
| import styles from './shared.module.css'; | ||
|
|
||
| class Item extends PureComponent { | ||
| render() { | ||
| const { data, index, style } = this.props; | ||
|
|
||
| return ( | ||
| <div | ||
| className={index % 2 ? styles.ListItemOdd : styles.ListItemEven} | ||
| style={style} | ||
| > | ||
| {data} {index} | ||
| </div> | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| export default function() { | ||
| return ( | ||
| <div className={styles.ExampleWrapper}> | ||
| <h1 className={styles.ExampleHeader}>Basic List</h1> | ||
| <div className={styles.Example}> | ||
| <ProfiledExample | ||
| className={styles.ExampleDemo} | ||
| sandbox="fixed-size-list-vertical" | ||
| > | ||
| <FixedSizeList | ||
| className={styles.List} | ||
| height={150} | ||
| itemCount={1000} | ||
| itemData="صف" | ||
| itemSize={35} | ||
| width={300} | ||
| style={{ direction: "rtl" }} | ||
| > | ||
| {Item} | ||
| </FixedSizeList> | ||
| </ProfiledExample> | ||
| <div className={styles.ExampleCode}> | ||
| <CodeBlock value={CODE_VERTICAL} /> | ||
| </div> | ||
| </div> | ||
| <div className={styles.Example}> | ||
| <ProfiledExample | ||
| className={styles.ExampleDemo} | ||
| sandbox="fixed-size-list-horizontal" | ||
| > | ||
| <FixedSizeList | ||
| className={styles.List} | ||
| direction="horizontal" | ||
| height={75} | ||
| itemCount={1000} | ||
| itemData="عمود" | ||
| itemSize={100} | ||
| width={300} | ||
| style={{ direction: "rtl" }} | ||
| > | ||
| {Item} | ||
| </FixedSizeList> | ||
| </ProfiledExample> | ||
| <div className={styles.ExampleCode}> | ||
| <CodeBlock value={CODE_HORIZONTAL} /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about parsing the style in this way. Seems like RTL pages will likely have a
direction: rtlon the level of the body rather than the list. Normally I'd say an explicitdirectionprop would be better, but that's already used in the list to differentiate between "horizontal" and "vertical". Hmm...Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In CSS this is called
directionbut that's already in use for lists. Ideally I would have maybe used "layout" or something but that ship has sailed, and I'm not really willing to do a major release just to rename this property.There's a related HTML attribute called
dirbut that doesn't really follow the prop naming convention.I briefly considered maybe using
directionanyway and just expanding it to be "vertical", "horizontal", or "rtl" (which would be horizontal with RTL direction) but that's maybe...not the most intuitive.I am currently leaning toward an
isRTLboolean attribute, even though...that's a bit weird too.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe
directioncan accept multiple values, e.g. "vertical ltr" (default), "horizontal ltr", "horizontal rtl", etc.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or maybe I should just add a new
layoutprop and deprecatedirection(with a DEV warning) for now. I think this is maybe the best long-term option.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my use case I saw the usage of direction (CSS) in an isolated fashion (component biased). But in a real scenario is true that this can be affected by parent styles. Definitely the last approach seems the way to go.