Enhance TextInput suggestions to inform button if mouse or keyboard #7576
Conversation
storybook/preview.js
Outdated
|
|
||
| const hpeNext = deepMerge(hpe, { |
There was a problem hiding this comment.
These changes are just for PR review, should be removed before merge.
| @@ -464,6 +469,7 @@ const MaskedInput = forwardRef( | |||
| kind={!child ? 'option' : undefined} | |||
| hoverIndicator={!child ? undefined : 'background'} | |||
| label={!child ? option : undefined} | |||
| keyboard={!mouseMovedSinceLastKey} | |||
There was a problem hiding this comment.
If we add this, we don't have to document immediately, but thinking ahead to how this could be documented would be:
keyboard?: boolean // Whether keyboard is being used to navigate.
|
It's currently tied to keyboard navigation, but should it instead be a state like maybe the button |
I like the idea of that. In this case "active" needs to be true for both mouse and keyboard. In the case of mouse, it'll just style with the kind-defined "active" styles (in HPE theme this is the grey background). In the case of keyboard, we want to have an extra decorator. --> so we kind of need 2 versions of active styling. But maybe we can expand what "active" can be set to: Then we don't have to pass a new prop. The only thing that is slightly awkward is the term "active" is a little vague in Grommet (something we should reconsider in v3). At times, it's the style applied when a button is "focused" (or visually focused/hovered in the case of textinput suggestions) and other time we use it for a more persistent state (like which nav item is currently selected). NOTE: After further discussion, have moved away from this approach. PR description has been updated with latest direction and rationale. |
This reverts commit d63aaf8.
What does this PR do?
An approach to allow themes to style in extend based on use of keyboard or mouse navigation. The approach passes
keyboardto Button to indicate if keyboard is currently being used as the means of navigating.Had explored extending the value of
activebut this seemed less than ideal sinceactiveis already slightly "messy" in its meaning in grommet (sometimes it's referring to a persistently active state that caller might set, and other times it's referring to when it has focus or mouse over). To avoid further cluttering/confusing the prop meaning in a more "global" way, introducing this as a separate prop.Given recent discussions around next steps for Grommet, it seems reasonable that we could have this as an undocumented prop for the time being so we can use it in the theme.button.extend in grommet-theme-hpe then revisit the broader pattern of mouse vs keyboard "active"-ness and how this should be addressed as a theme object/component logic when we might be able to have breaking changes.
Where should the reviewer start?
What testing has been done on this PR?
How should this be manually tested?
Do Jest tests follow these best practices?
screenis used for querying.asFragment()is used for snapshot testing.Any background context you want to provide?
What are the relevant issues?
Closes #7550
Screenshots (if appropriate)
Screen.Recording.2025-04-07.at.3.54.07.PM.mov
Do the grommet docs need to be updated?
Should this PR be mentioned in the release notes?
Is this change backwards compatible or is it a breaking change?