patternfly / patternfly-react

A set of React components for the PatternFly project.
https://react-staging.patternfly.org/
MIT License
762 stars 349 forks source link

Bug - Toolbar - page scrolls when controls are clicked #10511

Closed dlabrecq closed 5 days ago

dlabrecq commented 1 month ago

Discovered an issue with the latest PatternFly milestone packages in Cost Management, which is blocking QE test automation.

When users click the pagination's page selector and/or the bulk select, the page scrolls so that the control's menu is out of view. These controls do not behave like this when located outside the Toolbar. For example, the bottom pagination controls work as expected.

To reproduce with PatternFly's table demo, simply add the div below to create some height above the toolbar.

Modified table demo Screenshot 2024-06-03 at 1 05 27 PM

Sandbox https://codesandbox.io/p/sandbox/toolbar-scroll-issue-forked-9sn9yf?file=%2Findex.html

Steps

  1. Click on the pagination page selector and choose "100 per page".
  2. Click on the pagination page selector or the bulk select. You may need to click the control more than once.
  3. The page will scroll so the control's menu is out of view.

Cost Management

pagination scrolling

adamviktora commented 1 month ago

I did play around with this and it seems like the issue is caused by setting position: relative; in CSS in .pf-v5-c-toolbar and .pf-v5-c-toolbar__content classes.

mattnolting commented 1 month ago

I suspect popper applying position: absolute where pf-menu isn't present in the DOM scrolls the page. Let's try hiding the menu instead of removing it!

thatblindgeye commented 1 week ago

@dlabrecq I noted in my draft PR, but so far one solution that could be handled on our end ends up re-introducing a visual bug (the Dropdown menu flickers when rendering).

A (possibly, depending on the number of instances there are) quick solution would be from the consumer side, updating the Dropdown/Select being used in this sort of scenario to pass in popperProps={{appendTo: () => document.body}}. Would this at all be a viable workaround for now?

dlabrecq commented 1 week ago

@thatblindgeye I don't believe that solution will work for us. Considering the page selector is output by Pagination, we don't appear to have access to the underlying component.