-
Notifications
You must be signed in to change notification settings - Fork 15
fixed async pagination bugs #3
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
base: feature/async-pagination
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -21,7 +21,7 @@ const propTypes = { | |
| onChange: PropTypes.func, // onChange handler: function (newValue) {} | ||
| onInputChange: PropTypes.func, // optional for keeping track of what is being typed | ||
| options: PropTypes.array.isRequired, // array of options | ||
| pagination: PropTypes.bool, // automatically load more options when the option list is scrolled to the end; default to false | ||
| pagination: PropTypes.bool, // automatically load more options when the option list is scrolled to the end; default to false | ||
| placeholder: PropTypes.oneOfType([ // field placeholder, displayed when there's no value (shared with Select) | ||
| PropTypes.string, | ||
| PropTypes.node | ||
|
|
@@ -63,6 +63,7 @@ export default class Async extends Component { | |
|
|
||
| this.onInputChange = this.onInputChange.bind(this); | ||
| this.onMenuScrollToBottom = this.onMenuScrollToBottom.bind(this); | ||
| this.onChange = this.onChange.bind(this); | ||
| } | ||
|
|
||
| componentDidMount () { | ||
|
|
@@ -104,6 +105,10 @@ export default class Async extends Component { | |
| !pagination || | ||
| (pagination && (cache[inputValue].page >= page || cache[inputValue].hasReachedLastPage)) | ||
| ) { | ||
| this.setState({ | ||
| isLoading: false, | ||
| isLoadingPage: false | ||
| }); | ||
| return; | ||
| } | ||
| } | ||
|
|
@@ -178,8 +183,11 @@ export default class Async extends Component { | |
| onInputChange(transformedInputValue); | ||
| } | ||
|
|
||
| let oldInputValue = this.state.inputValue; | ||
| this.setState({ inputValue }); | ||
| this.loadOptions(transformedInputValue); | ||
| if (inputValue !== oldInputValue) { | ||
| this.loadOptions(transformedInputValue); | ||
| } | ||
|
|
||
| // Return the original input value to avoid modifying the user's view of the input while typing. | ||
| return inputValue; | ||
|
|
@@ -208,8 +216,19 @@ export default class Async extends Component { | |
| this.loadOptions(inputValue, this.state.page + 1); | ||
| } | ||
|
|
||
| onChange(value) { | ||
| this.props.onChange(value); | ||
|
|
||
| if (this.props.pagination) { | ||
| let remainingOptions = this.state.options.length - value.length; | ||
|
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. What is
Author
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. ah, you're correct. these fixes were specifically for my use case which was multi-select async pagination. let me test out some of the other edge cases and get back to you with another commit. |
||
| if (remainingOptions < 4) { | ||
| this.onMenuScrollToBottom(this.state.inputValue); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| render () { | ||
| const { children, loadingPlaceholder, multi, onChange, placeholder, value } = this.props; | ||
| const { children, loadingPlaceholder, multi, placeholder, value } = this.props; | ||
| const { isLoading, isLoadingPage, options } = this.state; | ||
|
|
||
| const props = { | ||
|
|
@@ -225,6 +244,7 @@ export default class Async extends Component { | |
| isLoading, | ||
| onInputChange: this.onInputChange, | ||
| onMenuScrollToBottom: this.onMenuScrollToBottom, | ||
| onChange: this.onChange | ||
| }); | ||
| } | ||
| } | ||
|
|
||
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.
setting state is async. This with the check two lines below will lead to bugs where the loadOptions is not called when it should have been.