-
Notifications
You must be signed in to change notification settings - Fork 1
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
RMAC-8398 - Fixed scroll issues #29
Conversation
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.
Please link the PR from react-select with the scroll change if you can
src/internal/ScrollCaptor.js
Outdated
|
||
// all the if statements are to appease Flow 😢 | ||
if (typeof el.addEventListener === 'function') { | ||
el.addEventListener('wheel', this.onWheel, false); | ||
if (typeof el.addEventListener === 'function' && !this.listeningToScroll) { |
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.
nitpick: double spaces
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.
Done.
src/internal/ScrollCaptor.js
Outdated
} | ||
if (typeof el.addEventListener === 'function') { | ||
if (typeof el.addEventListener === 'function' && !this.listeningToScroll) { |
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.
this should be listeningToTouchStart, right?
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.
Yep, fixed.
src/internal/ScrollCaptor.js
Outdated
componentWillUnmount() { | ||
this.stopListening(this.scrollTarget); | ||
} | ||
startListening(el: HTMLElement) { | ||
refreshListening(el: HTMLElement) { | ||
// bail early if no scroll available |
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 we should also bail early if we are already listening to everything
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.
Yep, done.
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.
Include package-lock as well
Fixed two issues:
onMenuScrollToBottom
event wasn't fired when using the mouse button to drag the scroll bar to the end of the menuonMenuScrollToBottom
event wasn't fired when the content (the items for the select) were loaded after the component was mounted.