[Scrollable] Fix scrolled to bottom checking for sub-pixel values

[rorz]

WHY are these changes introduced?

Fixes #3607, Fixes #3225

WHAT is this pull request doing?

Certain resolutions, OSes, and browser zoom-level combinations cause scrollTop to be a sub-pixel value (StackOverflow example), which does not satisfy the strict-equality check on the hasScrolledToBottom calculation of the Scrollable component.

In order to catch these cases, and fire the onScrolledToBottom callback prop, I've made the strict-equality check a less-than-or-equal to operator.

Notes:

• "less-than" might seem like a counter-intuitive operator here, but I didn't want to modify the original calculation and it is the appropriate comparison
• I experimented using Math.ceil(scrollTop) to catch sub-pixel values that are under the threshold, but this introduces another issue whereby scrolling up from the "at bottom" state can cause a re-fire of the callback. In testing, it seems to me that clientHeight is increased by a pixel at zoom levels where scrollTop is under the threshold anyway.

You can reproduce the original issue on Mac / Chrome by setting your browser zoom level to 67%, 75%, or 125%.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:

Important — To emulate a breaking resolution, in certain cases you need to set the browser zoom level (e.g. Chrome on Mac) to 67%, 75%, or 125%...

import React, {useState} from 'react';

import {Page} from '../src';
import {Scrollable} from '../src/components';

export function Playground() {
  const [atBottom, setAtBottom] = useState(false);
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
      <Scrollable
        style={{height: 100, background: 'yellow'}}
        onScrolledToBottom={() => setAtBottom(true)}
      >
        <div
          style={{
            background: 'blue',
            height: 150,
          }}
        />
      </Scrollable>
      {atBottom && (
        <h1
          style={{color: 'red', background: '#ccc'}}
          onClick={() => setAtBottom(false)}
        >
          BOTTOMED_OUT (click to reset)
        </h1>
      )}
    </Page>
  );
}

🎩 checklist

• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility
• Updated the component's README.md with documentation changes
• Tophatted documentation changes in the style guide
• For visual design changes, pinged one of @ HYPD, @ mirualves, @ sarahill, or @ ry5n to update the Polaris UI kit

[ghost]

👋 Thanks for opening your first pull request. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines.

[anasyusef]

@rorz I applied your patch to check how it would work out. I found a small issue with the calculation. For some values of scrollTop, it went over clientHeight by a slight amount, which would cause the condition to fail. To get around it, I just added 1 to scrollTop, to account for that amount. Check out the screenshot attached.

[alex-page]

@rorzis this still an issue? I can't replicate it with this codesandbox. https://codesandbox.io/s/floral-sun-rzebp1?file=/index.js

Feel free to reopen if it is an issue.