css: Fix PF dropdown elements not being accessible #22413
Conversation
Venefilyn
force-pushed
the
fix/dropdown-scroll
branch
from
ce1ef40 to
5d622a9
Compare
|
Tested on anaconda mode and it works as well! |
For Patternfly's dropdown it uses PopperJS v2 under the hood, however it does not work well within iframes with super small sizes where there is no room to show the whole dropdown. Fixing this temporarily by making the dropdown increase the page main sections height so that the dropdown elements can all be accessed by scrolling. Note, once a PF update is released with the PopperJS offset prop being exposed we can use a better solution that relies on setting our own offset. Related-to: patternfly/patternfly-react#11987 Related-to: cockpit-project#22381 Signed-off-by: Freya Gustavsson <[email protected]>
Venefilyn
force-pushed
the
fix/dropdown-scroll
branch
from
5d622a9 to
72fbe34
Compare
|
I'm on PTO tomorrow, Tuesday. Tests stopped working due to distrobox/toolbox usage and the image @mvollmer or @martinpitt can you take a look tomorrow why the tests fail? I suspect it to be related to https://logs-cockpit.apps.ocp.cloud.ci.centos.org/pull-22413-72fbe34e-20250908-163930-fedora-42-storage/log.html |
|
The screenshot shows that didn't work, i.e. there's no dropdown. That test works fine locally, both with firefox and chromium, and also with TEST_SHOW_BROWSER=1 and noninteractive. Trying to run it exactly like in CI: and that finally fails. So there's some "pollution" going on by previous tests. Bisect, rinse, repeat finally gives After the initial two LVM tests, the testMultiDevice failure is stable. The firefox failure is also revealing:
So it feels like the popup really isn't visible at the place of the dropdown. This even reproduces with This feels like fallout from appending the menu to the very bottom of the page ( |
|
In other words, shouldn't the position be set to the element that triggered the popup, not the target body/page? |
Unfortunately that creates two scrollbars |
|
@Venefilyn I didn't mean the logical DOM structure, but the |
There was a problem hiding this comment.
I'm approving, as it's better than what's there before and it's critical. Thanks! 🥳
I wonder if adding something like this in the popup code (tied to the element itself):
element.scrollIntoView({ behavior: "smooth", block: "nearest" });(It should only scroll if it needs to scroll.)
Might help to make sure that the menu is on screen in a best effort, so there's minimal scrolling needed to actually see the menu. It should try to fit it and stop when the top is on screen, probably. (Or "nearest" might need to be "start" instead. But "start" might just say "well, the start is on screen". Unsure; we'd need to test.)
https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoView
(Perhaps it'd only work as intended if it is set to the row, if the menu is a child to it, to make sure the row is onscreen, as is the origin button, and also tries a best effort for the menu.)
This could be a follow-up. (Or it might not even work. But it could improve the experience on smaller screens, especially when scrolling isn't obvious.)
5 participants
For Patternfly's dropdown it uses PopperJS v2 under the hood, however it
does not work well within iframes with super small sizes where there is no
room to show the whole dropdown. Fixing this temporarily by making the
dropdown increase the page main sections height so that the dropdown
elements can all be accessed by scrolling.
Note, once a PF update is released with the PopperJS offset prop being
exposed we can use a better solution that relies on setting our own
offset.
How the fix looks in action:
Kooha-2025-09-03-16-23-53.webm
el.querySelectorreturnsnullif the query is valid and no elements were found:https://developer.mozilla.org/en-US/docs/Web/API/Document/querySelector#return_value
Related-to: patternfly/patternfly-react#11987
Related-to: #22381
Signed-off-by: Freya Gustavsson [email protected]