-
Notifications
You must be signed in to change notification settings - Fork 176
Support delayed stabilisation for simple list #167
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: master
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 |
---|---|---|
|
@@ -57,6 +57,7 @@ module.exports = class ReactList extends Component { | |
minSize: PropTypes.number, | ||
pageSize: PropTypes.number, | ||
scrollParentGetter: PropTypes.func, | ||
stableFrameDelay: PropTypes.number, | ||
threshold: PropTypes.number, | ||
type: PropTypes.oneOf(['simple', 'variable', 'uniform']), | ||
useStaticSize: PropTypes.bool, | ||
|
@@ -90,6 +91,7 @@ module.exports = class ReactList extends Component { | |
|
||
componentWillReceiveProps(next) { | ||
let {from, size, itemsPerRow} = this.state; | ||
clearTimeout(this.updateFrameTimeoutId); | ||
this.maybeSetState(this.constrain(from, size, itemsPerRow, next), NOOP); | ||
} | ||
|
||
|
@@ -100,6 +102,7 @@ module.exports = class ReactList extends Component { | |
} | ||
|
||
componentDidUpdate() { | ||
let {stableFrameDelay, type} = this.props; | ||
|
||
// If the list has reached an unstable state, prevent an infinite loop. | ||
if (this.unstable) return; | ||
|
@@ -116,7 +119,14 @@ module.exports = class ReactList extends Component { | |
}, 0); | ||
} | ||
|
||
this.updateFrame(); | ||
if (type === 'simple' && stableFrameDelay) { | ||
this.updateFrameTimeoutId = setTimeout( | ||
this.updateFrame, stableFrameDelay | ||
); | ||
} else { | ||
this.updateFrame(); | ||
} | ||
|
||
} | ||
|
||
maybeSetState(b, cb) { | ||
|
@@ -129,6 +139,7 @@ module.exports = class ReactList extends Component { | |
window.removeEventListener('resize', this.updateFrame); | ||
this.scrollParent.removeEventListener('scroll', this.updateFrame, PASSIVE); | ||
this.scrollParent.removeEventListener('mousewheel', NOOP, PASSIVE); | ||
clearTimeout(this.updateFrameTimeoutId); | ||
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. Let's check if 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. Checking the timeout is superfluous (see MDN note). |
||
} | ||
|
||
getOffset(el) { | ||
|
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 don't think we need to restrict this prop to the
'simple'
list type. It seems like it could potentially be used for'variable'
or'uniform'
as well.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.
Well, based on my interpretation of the documentation, on
variable
you have to provide the height, so animating your elements should not interfere with it.About
uniform
, I've tried to enable it on but it didn't work out of the box. So, if you think it might be useful I might give it a second try (but I might need to touch more code).