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

Support Scroll User Event #475

Open
svadali2 opened this issue Oct 22, 2020 · 14 comments
Open

Support Scroll User Event #475

svadali2 opened this issue Oct 22, 2020 · 14 comments
Labels
needs specification The desired behavior is not defined yet new event New event to be included in the API

Comments

@svadali2
Copy link

svadali2 commented Oct 22, 2020

Problem description: When we have an area to scroll, it'd be good to account for this user event

Suggested solution: Add a scroll handler

@nickserv nickserv added needs investigation Someone has to do research on this new event New event to be included in the API labels Nov 8, 2020
@nickserv
Copy link
Member

nickserv commented Nov 8, 2020

We assume users use JSDOM, which doesn't implement most CSS layout features, so I'm not sure how useful it would be for a test to depend on what is scrollable/visible. Could you give a use case where a test (either using fireEvent or a proposed new API for userEvent) adds value for verifying scrolling behavior that isn't dependent on layout?

For example if you have an infinite scroll component, the test may or may not work properly depending on the size of scrolled elements. I may be missing other use cases that don't have this issue though.

@kentcdodds
Copy link
Member

Not everyone using this package is using jsdom (though most are). So I'm not opposed to adding things that really on layout. It just means we'd have to add a tool that supports layout for our own tests.

Personally, if I had sobering like this to test, I would just use cypress instead 🤷‍♂️

@nickserv
Copy link
Member

nickserv commented Nov 8, 2020

Does cypress-testing-library handle events using dom-testing-library or is it assumed that users will use Cypress commands directly?

@kentcdodds
Copy link
Member

It assumes people will interact with the DOM using cypress. No reason to implement that ourselves when cypress already did a great job of that. User event is a less good version of what cypress does.

@nickserv
Copy link
Member

nickserv commented Nov 8, 2020

In that case, I guess it comes down to if this is worth the effort to support without Cypress. If you have a test like this it's entirely possible that JSDOM isn't good enough and you'd need Cypress anyway, and I imagine cypress-testing-library should make it easier to migrate.

@svadali2
Copy link
Author

svadali2 commented Nov 9, 2020

I am in a situation where I cannot use cypress to test this out - it would bloat the codebase if only a single component uses the library. It would be highly preferred to be able to use jest, enzyme or rtl. How would you recommend someone be able to test out, say, an event that triggers at the end of a scroll action? With or without the given libraries as options.

@nickserv
Copy link
Member

nickserv commented Nov 9, 2020

You could mock whatever your code uses to detect scroll position. Or if that logic is in its own function you can replace the entire thing with a mock.

@ph-fritsche ph-fritsche added needs specification The desired behavior is not defined yet and removed needs investigation Someone has to do research on this labels Mar 16, 2021
@Anatoliy13Kravchenko
Copy link

Anatoliy13Kravchenko commented Dec 25, 2022

Hi. Actually, I've met a case when a user wheels a page under the focused input[type=number] and its value has changed. I added some code to avoid such behaviour, but I also wanted to cover it with the test but I couldn't. I hope, it would be useful

@xsjcTony
Copy link

xsjcTony commented Jan 21, 2023

+1 for above, encountering a use case of scrolling on <input type="number">

More specifically, I want to test the onWheel event but seems like I've no way to do it at the moment.

@julianCast
Copy link

julianCast commented Feb 6, 2023

Hi! Another use case:
A UI where a Continue button is disabled, waiting for the user to scroll to the bottom of the list (within a fixed-height container).

@npeterkamps
Copy link

For a scrollbar component with an event for scrollToBottom, I also ran into the issue of layouting. We're not yet using Cypress within the repo, so I wanted to solve it with jsdom.

I found a StackOverlow answer by kamlesh suggesting using spyOn(element, 'scrollHeight', 'get').mockImplementation(() => scrollHeight) to mock the getter function. That way, you can manually mock the layouting values used by the scroll position calculations.

We also have a threshold, so you don't have to (necessarily) scroll all the way to the bottom, but within a range of it. Trying to test this behavior made the limitation of fireEvent pretty clear. I wrote a function to simulate the scroll event happening many times to scroll from one position to the next.

const scroll = (scrollBar, scrollTop) => {
  fireEvent.scroll(scrollBar, { target: { scrollTop } });
};

const simulateScroll = (scrollBar, scrollTop) => {
  let currentScrollTop = scrollBar.scrollTop;
  let change = scrollBar.scrollTop > scrollTop ? -10 : 10;
  if (Math.abs(scrollBar.scrollTop - scrollTop) % 100 === 0) {
    // performance optimization. Go 50/-50 at once when difference is a couple 100.
    change *= 5;
  }

  while (
    currentScrollTop !== scrollTop &&
    !(
      currentScrollTop < 0 ||
      currentScrollTop > scrollBar.scrollHeight - scrollBar.clientHeight
    )
  ) {
    currentScrollTop += change;
    scroll(scrollBar, currentScrollTop);
  }
};

Since this issue has the tag "needs specification", I'm wondering whether the above is something that this package wants to include. Then in the docs, we could give caveats and advice to use cypress, but still provide a helper if someone wants it. Could save some people some time.

@yaboi
Copy link

yaboi commented May 16, 2024

+1 for scrolling on <input type="number">, also more specifically testing the disabling of 'wheel' event.

@gabrielcosi
Copy link

+1

@HellRok
Copy link

HellRok commented Jul 30, 2024

+1, we specifically also want to test that a <input type="number"> doesn't increment when a wheel event is triggered on it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs specification The desired behavior is not defined yet new event New event to be included in the API
Projects
None yet
Development

No branches or pull requests