Only consider document scrollable if not prevented #1081
Conversation
🦋 Changeset detectedLatest commit: 62b5ded The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| export function isScrollable( | ||
| element: HTMLElement, | ||
| export function hasOverflowStyles( | ||
| element: Element, |
There was a problem hiding this comment.
HTMLElement is a subset of Element, so Element should be fine to use here (it won't break the public API).
Broadening the type was needed to accept node.scrollableElement in getScrollableAncestors.ts, which is of type Element and not HTMLElement. However they both are processed by hasOverflowStyles just fine, because getComputedStyle accepts both.
There was a problem hiding this comment.
Nit: it's a breaking change if somebody uses this function's type in contra-variant position like const fn: (hasOverflowStyles: typeof hasOverflowStyles) => void and then fn(myCustomHasOverflowStyles) but it's a stretch.
|
|
||
| import {hasOverflowStyles} from './hasOverflowStyles'; | ||
|
|
||
| export function isScrollingDisabled( |
There was a problem hiding this comment.
I created this dedicated function because using a single isScrollable function containing both checks doesn't work for the following reasons:
isScrollingEnabledneeds to check if scrolling was explicitly enabled.isScrollingDisabledneeds to check if scrolling was explicitly disabled.
Combining both into Boolean(isScrollingEnabled && !isScrollingDisabled) (in a single isScrollable function) would imply the element won't be considered scrollable unless it explicitly had scrolling enabled, which is not something that is wanted when checking node.scrollableElement in getScrollableAncestors.ts because that element should always be considered scrollable, unless scrolling is explicitly disabled.
At the moment, the
<html>element will always be added to the list of scrollable ancestors, even if the element has its scrolling ability deactivated usingoverflow: hidden.As a result, sensors like
KeyboardSensorwill mistakenly scroll<html>when dragging elements, causing unwanted glitches in the visual experience.