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

ScrollArea onButtonReached does not call with decimal values #6792

Closed
1 of 2 tasks
etwodev opened this issue Sep 9, 2024 · 6 comments
Closed
1 of 2 tasks

ScrollArea onButtonReached does not call with decimal values #6792

etwodev opened this issue Sep 9, 2024 · 6 comments
Labels
Fixed patch Completed issues that will be published with next patch (1.0.X)

Comments

@etwodev
Copy link

etwodev commented Sep 9, 2024

Dependencies check up

  • I have verified that I use latest version of all @mantine/* packages

What version of @mantine/* packages do you have in package.json?

^7.12.2

What package has an issue?

@mantine/core

What framework do you use?

Next.js

In which browsers you can reproduce the issue?

Chrome

Describe the bug

in ScrollArea there are instances where scrollTop is fractional, meaning onBottomReached will never be called - as this edge case is not handled, seen below:

          const { scrollTop, scrollHeight, clientHeight } = e.currentTarget;
          if (scrollTop - (scrollHeight - clientHeight) === 0) {
            onBottomReached?.();
          }

If scrollTop was a value such as 495.5, scrollHeight at 696 and clientHeight at 200, this would not call, despite being unable to scroll further.

If possible, include a link to a codesandbox with a minimal reproduction

https://codesandbox.io/p/sandbox/mantine-react-template-forked-wtx8tg

Possible fix

Replace

          const { scrollTop, scrollHeight, clientHeight } = e.currentTarget;
          if (scrollTop - (scrollHeight - clientHeight) === 0) {
            onBottomReached?.();
          }

with

          const { scrollTop, scrollHeight, clientHeight } = e.currentTarget;
          if (Math.abs(scrollHeight - clientHeight - scrollTop) < 1) {
            onBottomReached?.();
          }

Self-service

  • I would be willing to implement a fix for this issue
@etwodev etwodev changed the title ScrollArea onButtonReached does not always call with decimal values ScrollArea onButtonReached does not call with decimal values Sep 9, 2024
@Kenzo-Wada
Copy link
Contributor

Kenzo-Wada commented Sep 11, 2024

I didn't reproduce on codesandbox environment, Has Reached Bottom become true 😢

@etwodev
Copy link
Author

etwodev commented Sep 11, 2024

I didn't reproduce on codesandbox environment, Has Reached Bottom become true 😢

Can confirm this issue doesn't seem present in Firefox, not sure about Safari, but it seems reproducible in Chrome
Possibly only macOS chrome...

@Kenzo-Wada
Copy link
Contributor

I see, I'll check it again on macos thanks! I was using Ubuntu...

@etwodev
Copy link
Author

etwodev commented Sep 11, 2024

I see, I'll check it again on macos thanks! I was using Ubuntu...

Also realised my example was wrong, issue should persist in Firefox now :)

But it is likely due to the fact it's dependent on your device

Regardless, the experience is super inconsistent and doesn't ignore the fact that scrollTop can be decimal:
https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollTop

rtivital added a commit that referenced this issue Oct 3, 2024
@rtivital rtivital added the Fixed patch Completed issues that will be published with next patch (1.0.X) label Oct 3, 2024
@rtivital
Copy link
Member

rtivital commented Oct 3, 2024

Fixed in 7.13.2

@rtivital rtivital closed this as completed Oct 3, 2024
@AlexeyEsin
Copy link

AlexeyEsin commented Oct 17, 2024

It still doesn't work in some cases. In my case, when using MultiSelect with 24 entries in data, onBottomReached does not work in Chrome.

https://codesandbox.io/p/sandbox/mantine-react-template-forked-g7jynq

I tried the solution presented by @etwodev and it worked. I passed the onScroll callback to viewportProps, which is in scrollAreaProps, and the log is triggered just like onBottomReached should be triggered.

const onScroll = (e: UIEvent) => {
  const { scrollTop, scrollHeight, clientHeight } = e.currentTarget;
  if (Math.abs(scrollHeight - clientHeight - scrollTop) < 1) {
    console.log('bottom reached');
  }
};

Therefore, it seems that this solution is more correct than the current one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fixed patch Completed issues that will be published with next patch (1.0.X)
Projects
None yet
Development

No branches or pull requests

4 participants