Skip to content
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

Merged
merged 6 commits into from
Dec 29, 2021
Merged

Conversation

OS-rafaelduarte
Copy link

Fixed two issues:

  • The onMenuScrollToBottom event wasn't fired when using the mouse button to drag the scroll bar to the end of the menu
  • The onMenuScrollToBottom event wasn't fired when the content (the items for the select) were loaded after the component was mounted.

@OS-rafaelduarte OS-rafaelduarte added the bug Something isn't working label Dec 28, 2021
Copy link

@marianacapelo marianacapelo left a 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


// 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) {

Choose a reason for hiding this comment

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

nitpick: double spaces

Copy link
Author

Choose a reason for hiding this comment

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

Done.

}
if (typeof el.addEventListener === 'function') {
if (typeof el.addEventListener === 'function' && !this.listeningToScroll) {

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?

Copy link
Author

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 Show resolved Hide resolved
componentWillUnmount() {
this.stopListening(this.scrollTarget);
}
startListening(el: HTMLElement) {
refreshListening(el: HTMLElement) {
// bail early if no scroll available

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

Copy link
Author

Choose a reason for hiding this comment

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

Yep, done.

Copy link

@marianacapelo marianacapelo left a 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

@OS-rafaelduarte OS-rafaelduarte merged commit 56b290f into main Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants