react-spectrum icon indicating copy to clipboard operation
react-spectrum copied to clipboard

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

Open jeffijoe opened this issue 1 year ago โ€ข 5 comments

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:

https://github.com/user-attachments/assets/1b748c0a-61f0-413f-a3b1-995879c1fc54

๐Ÿ’ 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:

image

๐Ÿ–ฅ๏ธ 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

jeffijoe avatar Sep 14 '24 18:09 jeffijoe

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

snowystinger avatar Sep 16 '24 04:09 snowystinger

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.

LFDanLu avatar Sep 16 '24 17:09 LFDanLu

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

jcorkhill avatar Sep 16 '24 21:09 jcorkhill

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

deebov avatar Sep 21 '24 16:09 deebov

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 avatar Oct 03 '24 17:10 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.

jeffijoe avatar Nov 29 '24 13:11 jeffijoe

My guess is this: https://github.com/adobe/react-spectrum/blob/7eae25e124a9d235ff30f12bd86383b894a1ea04/packages/%40react-aria/utils/src/scrollIntoView.ts#L87-L89

If scroll locking is enabled, this alternate path is used: https://github.com/adobe/react-spectrum/blob/7eae25e124a9d235ff30f12bd86383b894a1ea04/packages/%40react-aria/utils/src/scrollIntoView.ts#L102-L106

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 https://github.com/w3c/csswg-drafts/issues/9452). So I guess we would need to adjust the calculation in that function to take scrollPadding into account.

devongovett avatar Dec 05 '24 03:12 devongovett

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?

jeffijoe avatar Dec 05 '24 10:12 jeffijoe

Sure, that'd be great

devongovett avatar Dec 05 '24 16:12 devongovett

@devongovett I've opened PR https://github.com/adobe/react-spectrum/pull/7484 with a fix, thanks for pointing me in the right direction.

jeffijoe avatar Dec 05 '24 18:12 jeffijoe