[@mantine/hooks]: fix unstable pinning of item by useHeadroom on mobile devices

[Alimobasheri]

fixes #4563

In order to fix the issue, I actually had to change the code logic for checking pin status. The isPinned function was previously used every time the scrollPosition returned by useWindowScroll changed. This is useless as causes the issue as when an address bar hides on scroll in mobile browser, a resize event is fired by the browser, but the scrollPosition hasn't actually changed.

if (isPinned(scrollPosition, scrollRef.current)) {
    return true;
  }

In the above code, both scrollRef.current and scrollPosition hold the same value and don't offer a clear comparison.
This line of code is useless because the next condition call after it which calls isFixed with scrollPosition and the fixedAt value is the only true logic for this hook's implementation.

if (isFixed(scrollPosition, fixedAt)) {
    return true;
  }

I also changed the logic for calling onPin and onRelease callbacks. The ref which was previously used to hold the scrollPosition was actually used for wrong comparison. The onPin callback in my opinion should get called only when the pinning element has switched state from pinned to unpinned or vice versa, when the scrollPosition changes. In the current code, onPin is getting called so many times as user scrolls, this doesn't make sense. So, I have merged the logic of two past useEffects into a single one. I store the pinning status in a ref to prevent extra re-render.

I have marked this PR as a draft to make sure you inspect those changes and I hope this explanation clarifies them for you.

[rtivital]

Thanks for sending a PR! Unfortunately, it does not work. The header does not become visible when the page is scrolled up.

2024-02-26.17.26.47.mov

[Alimobasheri]

@rtivital you're right. I hadn't paid attention to this part of the doc. But the old solution for this, conflicts with mobile device's resize event when the address bar hides. The user hasn't actually scrolled up in that case. Because this particular resize event doesn't change the scrollPosition or the top position of it.
A solution can be adding another hook which returns the scroll direction based on these conditions: 1. A scrollEvent has happened (not a resize one) 2. And the user is still scrolling up
I mean the current code is tricky to be fixed, since it still has to take resize event into consideration for the fixed position.

And do you agree that the current code calls the onPin event excessively? Or is that part of the PR invalid too?

[rtivital]

I haven't really explored this issue. The hook was contributed by other community member, so it should either be fixed or rewritten from scratch.

[Alimobasheri]

So I will try to update this PR with the fix for the issue you mentioned.

[Alimobasheri]

@rtivital I've pushed a new commit which fixes the issue with scroll up.

I had to add the useScrollDirection hook specifically in the code to make sure the user is scrolling up. And I had to add a debounce for resize events in that hook.
This debounce makes sure the scrollTop isn't changed by a resize event (like the address bar hiding) and so the user is actually scrolling up.

[rtivital]

Thanks! Works correctly on mobile