-
Notifications
You must be signed in to change notification settings - Fork 353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug - Menus (dropdown, menutoggle, etc.) - first menu item in a dropdown has a highlight by default (and it shouldn't) #10393
Comments
@garrett thanks for bringing this to our attention! For what it's worth, the first item upon clicking open a toggle is being focused rather than being styled to look hovered. The focus and hover styles are the same, though, which may be adding to this confusion. This functionality was added in React in a PR to focus the first item when opened by click, which helped resolve an issue regarding not being able to traverse the menu items via keyboard after clicking the toggle. With that in mind does that help alleviate the concern with this behavior, or is the behavior still unexpected and/or confusing? |
This also seems to cause a problem with fields that allow text input via the keyboard. Previously, we were able to click directly into the field and filter the displayed elements via keyboard input. This is now only possible after an additional click in the field. You can easily see that by comparing the UX of https://www.patternfly.org/components/menus/select/#typeahead to: |
Any news on this being fixed? We still see this all the time in Cockpit, and would love for this regression to be fixed... and in PF5 (where it was introduced in April), not just PF6. See this dropdown where the first one is selected and the one at the bottom has a highlight: It's really quite bad and confusing to have the same focus and hover stylings and have two items selected at the same time. |
@garrett we currently have #10642 open for v5 that will allow disabling the auto-focus, but will require manual logic to focus an item (which will be needed when the dropdown is opened via keyboard at the very least). This would more help resolve scenarios where a dropdown may have something other than dropdown items (e.g. an input before the dropdown items) that should receive focus first. The issue with the hover/focus styling being the same may not be something we land for v5 since it's a topic that stretches beyond just Dropdown styling, though. We also have #10543 for a post-v6 spike to investigate a more robust/customizable way to handle the autofocus, though again that wouldn't address the styling being the same for hover/focus. |
Thanks for the response!
It's a very visible, confusing regression that happened in a v5 point release. Projects cannot simply update to v6, as it introduces all kinds of breakages, including a radically different style that has fonts that are illegibly small. It will break all visual tests for projects too, so we're all at risk of having bugs slip through uncaught. (There will simply be way too many screens to compare to catch everything, without investing tons of time and hoping that everything is caught and addressed, including combinations of widgets the PatternFly team doesn't test together.) Back to the issue filed here: This isn't one of the many bugs that can be papered over with local CSS overrides — it requires fixes in JavaScript, which means that all of the projects using PatternFly are depending on a fix from PatternFly directly. This really is something that should be addressed in v5. |
Does #10642 aid in addressing this for v5? The PR adds a prop to prevent that auto-focus behavior. Or is the ask more to only prevent that auto-focus behavior when using a mouse to click open the Dropdown (matching the behavior previously seen in Dropdown)? |
@thatblindgeye: Thanks for looking into this! The proposed solution doesn't fix the issue; it just adds a workaround. The default behavior should be correct without having to do anything special. Relying on a workaround means developers need to know about the problem and add extra code, which isn't ideal. This bug needs a direct fix. For menus and menu-based components:
This issue seems to stem from a code change that prioritized keyboard interactions over mouse interactions. Keyboard and mouse actions should behave differently, and their visual cues for focus and highlight should also differ. (Currently, in PatternFly 5, they look the same. But this bug should be able to be fixed without changing the style.) Fixing this in PatternFly 5 is quite imporant as software using this version will be in many enterprise products for around a decade. Upgrading to PatternFly 6 will require significant changes and moving away from deprecated components, so many projects will not do this in the immediately and will be stuck with PatternFly 5 (and this confusing issue, unless fixed) for a very long time. |
The reason for this behavior is due to the change made for #9788, which we feel also helps alleviate an accessibility issue. While the original issue was more of a general ask for multi-device support and the intent of the PR was to prioritize combo mouse+keyboard interaction rather than solely one or the other (e.g. mouse click a menu toggle, but be able to immediately navigate a menu via keyboard arrow keys), in some testing we found that certain AT looked to benefit from this behavior as well; where voice input is used to command actions, stating "click"/"touch" on the toggle acted similarly to a native mouse click, and without auto-focusing the menu content it could be very tedious and require more-than-necessary effort to get into the menu/click a menu item. While it's a bit confusing at first that the first item in a Dropdown menu has that focus styling and another item will have the hover styling when hovered via mouse, the behavior itself is intended. It's unfortunate that we're relying on native browser focus outlines which causes both a focused item and hovered item to have the same styling under certain circumstances, and that there's some odd browser behavior happening where:
It's worth noting that the behavior where 1 item can have hover style and another item can have focus styles predates this recent behavior, and I don't believe these components fully mimicked a native Depending on bandwidth this may be something we could look into addressing for v5 after v6 is released, though @nicolethoen may have more insight on that once we get further to the v6 release. |
Right, which is one of the several reasons why I strongly feel that a proper fix is to not have an option to disable it. I looked at Windows, macOS, GNOME, and KDE and no other widget set does this (and for good reason): Video: Kooha-2024-07-04-16-19-49.webm(It finally lost the style when I clicked outside of the browser.) In fact, out of the survey of all OSes and desktops, only GNOME (the desktop we ship in RHEL) preselects, but it importantly acts differently than what PatternFly does: Kooha-2024-07-04-16-23-20.webmNote that when you're using the mouse and hover over something else, the selection style actually changes? You don't get two rows that have a highlight. When you use the keyboard, it also moves the selection. It looks the same on focus or hover, but there's only ever one. They made it so that it adapts and does the expected thing when your using a keyboard vs a mouse. Additionally, when their menus have other widgets that are within the menu that aren't just standard text-based menuitems, their backgrounds are not preselected or hovered (the widgets get their own native focus ring when using keyboard focus): Kooha-2024-07-04-16-28-33.webmTo be clear, this exactly what I'm asking for:
While this isn't 100% what Windows or macOS do as they do not preselect, what I'm asking for happens to be what all GNOME and GTK apps do in RHEL, which is what we're shipping at Red Hat too, so there's a precedence for how this should be done and we're already doing it correctly elsewhere. Currently, PatternFly has a half-baked implementation that does not work as expected, as the well-intentioned accessibility commit did not consider interaction with a mouse, nor did it consider menus that have anything other than menuitems with text only. I'm asking that this confusing visual regression be fully addressed in such a way that the applications that we ship to customers are not confusingly broken for the next decade due to this regression. Thanks. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
Still not stale, still a regression, still not fixed in PF5 (where it was introduced in a minor version on an update) and even seems to be a problem in PF6 as well, according to the staging site. As seen on https://staging-v6.patternfly.org/get-started/design/ — on any dropdown, even the one in the top-right: |
Describe the problem
This is an issue with
[?] Patternfly 6
Please give a clear and concise description of the problem. Which components are affected?
When clicking on a dropdown or menutoggle, the first item in a menu is already selected.
How do you reproduce the problem?
Open any dropdown, menutoggle, or other menu-based component and look at the first menu item. It looks like it's being hovered, but it isn't. This affects all the new menus on the PatternFly website as well.
Expected behavior
An item that isn't hovered or focused shouldn't look hovered or focused.
Is this issue blocking you?
We had this as part of a blocker for our release, as it's a regression from the previous (now deprecated) dropdowns and we recently ported to the new ones.
Screenshots
From PatternFly's website:
This affects Chrome too:
And also WebKit:
What is your environment?
What is your product and what release date are you targeting?
Cockpit. Yesterday was the release. We're unfortunately now shipping with this bug, as we cannot indefinitely postpone the release to wait on a fix in a PatternFly release.☹️
The text was updated successfully, but these errors were encountered: