scroll-padding not respected when ListBox is rendered in a Popover

[jeffijoe]

Provide a general summary of the issue here

When a ListBox which contains scroll-padding styles is rendered in a Popover, the scroll padding is not respected. For example, the contact list example uses and shows the usefulness of scroll padding, but if that contact list was to be rendered in a popover, it would result in a bad keyboard experience as the focused item may be obscured by a sticky header.

🤔 Expected Behavior?

Scroll padding should work regardless of whether the listbox is rendered in a popover.

😯 Current Behavior

The scroll padding during keyboard navigation is not respected, resulting in any sticky headers clipping the focused item.

The following video is from the code sandbox reproduction:

listbox-issue-encoded.mp4

💁 Possible Solution

I thought it was a general browser issue with scroll padding and having the scroll container hosted in a non-static container. I tried placing it in a simple div with position: absolute|fixed but the issue did not reproduce. It was only inside the popover I could reproduce it.

🔦 Context

My use case is a Select with sticky headers:

🖥️ Steps to Reproduce

See this code sandbox which includes both a reproduction and an option to render the list box outside a popover to observe that it works correctly otherwise.

Version

RAC 1.3.3, RA 3.34.3

What browsers are you seeing the problem on?

Chrome, Safari

If other, please specify.

No response

What operating system are you using?

macOS

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

[snowystinger]

@LFDanLu I think you've been looking at some scroll-padding issues in Table. Anything related to your findings here?

[LFDanLu]

https://issues.chromium.org/issues/365913982 which came from that Table investigation definitely shows up as an issue for when the ListBox isn't rendered in a Popover, but I'm not sure that is the reason for the "in popover" behavior that is being exhibited here since increases to the scroll-padding doesn't seem to affect the positioning at all still.

[jcorkhill]

I see the same erroneous behavior in Firefox Developer Edition 131.0b7 (aarch64).

[deebov]

We're having the issue too. Has anyone found a good workaround for this yet?

[jeffijoe]

I noticed scroll padding isn't working for us inside scrollable dialogs as well, perhaps related to #7131 - when focus moves around, the scroll padding I defined on the scroll container is not respected.

[jeffijoe]

In case it's helpful to the maintainers: I upgraded the packages in the sandbox to the latest released versions (3.36.3 and 1.5) and the issue persists.

[devongovett]

My guess is this:

react-spectrum/packages/@react-aria/utils/src/scrollIntoView.ts

Lines 87 to 89 in 7eae25e

	let isScrollPrevented = window.getComputedStyle(root).overflow === 'hidden';
	// If scrolling is not currently prevented then we aren’t in a overlay nor is a overlay open, just use element.scrollIntoView to bring the element into view
	if (!isScrollPrevented) {

If scroll locking is enabled, this alternate path is used:

react-spectrum/packages/@react-aria/utils/src/scrollIntoView.ts

Lines 102 to 106 in 7eae25e

	let scrollParents = getScrollParents(targetElement);
	// If scrolling is prevented, we don't want to scroll the body since it might move the overlay partially offscreen and the user can't scroll it back into view.
	for (let scrollParent of scrollParents) {
	scrollIntoView(scrollParent as HTMLElement, targetElement as HTMLElement);
	}

scrollIntoView is a custom implementation that we use to avoid scrolling the body while a modal/popover is open. Unfortunately the native scrollIntoView doesn't support that (see w3c/csswg-drafts#9452). So I guess we would need to adjust the calculation in that function to take scrollPadding into account.

[jeffijoe]

Ah interesting! I didn't know about the alternative path for scrolling - checking for scroll padding there seems like it would do the trick.

If it's as simple as reading the scroll padding here and adding it, then I could take a stab at it?

[devongovett]

Sure, that'd be great

[jeffijoe]

@devongovett I've opened PR #7484 with a fix, thanks for pointing me in the right direction.