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

[v2] onMenuScrollToBottom does not fire if dropdown is not scrollable on mount #2862

Closed
pyreta opened this issue Jul 27, 2018 · 6 comments
Closed
Labels
issue/bug-unconfirmed Issues that describe a bug that hasn't been confirmed by a maintainer yet issue/reviewed Issue has recently been reviewed (mid-2020) menu-bug Addresses menu positioning, scrolling, or general interactions

Comments

@pyreta
Copy link
Contributor

pyreta commented Jul 27, 2018

@JedWatson I wasn't sure how to test but you can see a codesandbox example here where opening the dropdown with options already loaded allows for the onMenuScrollToBottom callback to log "bottom" in the console. However if you clear the options before opening the dropdown, the callback will not fire after options have loaded.

I've submitted this PR as a fix.

Thanks!

@vtaits
Copy link

vtaits commented Sep 6, 2018

// src/internal/ScrollCaptor.js

  startListening(el: HTMLElement) {
    // bail early if no scroll available
    if (el.scrollHeight <= el.clientHeight) return; // <------- listening stops here

    // all the if statements are to appease Flow 😢
    if (typeof el.addEventListener === 'function') {
      el.addEventListener('wheel', this.onWheel, false);
    }
    if (typeof el.addEventListener === 'function') {
      el.addEventListener('touchstart', this.onTouchStart, false);
    }
    if (typeof el.addEventListener === 'function') {
      el.addEventListener('touchmove', this.onTouchMove, false);
    }
  }

What about add prop for allow listen scrolls always?

@KarolinaPru
Copy link

Hi, is @pyreta 's change going to be merged anytime soon? I'm having the same issues.

@macobo
Copy link
Contributor

macobo commented Jan 25, 2019

@JedWatson What's the next step to resolve this bug?

There's two existing PRs addressing this usecase, neither with clear feedback on what needs to be done differently to accept the PRs:

@bogdanpetru
Copy link

Any update on this issue?

To me this fix seems ok: #3219

We have this issue with paginated select component. If the select is opened before the first page is loaded the pagination is broken.

The fix I've used it is kinda hacky and I would like to get rid of it. I change the key on select-menu when first page is loaded. This makes the menu remount, menu as scroll and it listens to scroll events.

I am open to contribute to fix this issue.

Thank you!

Screenshot 2019-06-18 at 11 05 38

@bladey bladey added the issue/bug-unconfirmed Issues that describe a bug that hasn't been confirmed by a maintainer yet label May 29, 2020
@bladey bladey added the issue/reviewed Issue has recently been reviewed (mid-2020) label Jun 17, 2020
devwebpeanuts added a commit to naoned/react-select that referenced this issue Sep 22, 2020
@ebonow
Copy link
Collaborator

ebonow commented Mar 13, 2021

Greetings, everyone and apologies for the extremely late reply.

I recall looking at this earlier without being able to replicate the example so I decided to put a little more time and better understand the use-case. Since the root case is the options being loaded when the menu is already open, I modified the example above to load the options on setTimeout.

https://codesandbox.io/s/strange-goodall-nncgb?file=/src/index.js

  1. Click load options
  2. Click select and opening the Menu (within 5 seconds of clicking load options button)
  3. When options load, onScrollBottom event is not called

Perhaps this better encapsulates the root cause as pointed out by @vtaits .

The crux of all of this is that all of these events are applied to a ref to which should be just onScroll but instead iOS WebView in earlier implementations would not send the onScroll event until the scroll was completed.

So 3 possible solutions for this are:

  1. Apply the "scroll-ish" synthetic event handlers directly to the component instead of the nullable ref
  2. Add a dependency on scrollTarget.current in the hook to verify that startListening is only called when it's not null.
  3. Do a rewrite that replaces all of these events with an IntersectionObserver to better track changes in the scrollTarget scrolling position

My preference would like be the third option as it likely overcomes all of the challenges of listening to touch scrolling events.

@ebonow ebonow added the menu-bug Addresses menu positioning, scrolling, or general interactions label Jun 10, 2021
@Methuselah96
Copy link
Collaborator

I updated react-select to the latest in the CodeSandbox and I believe this issue has been resolved: https://codesandbox.io/s/interesting-microservice-dm8rk?file=/src/index.js. Closing this, feel free to comment if you think this is still an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/bug-unconfirmed Issues that describe a bug that hasn't been confirmed by a maintainer yet issue/reviewed Issue has recently been reviewed (mid-2020) menu-bug Addresses menu positioning, scrolling, or general interactions
Projects
None yet
Development

No branches or pull requests

8 participants