[DataGridPro] Fix custom header filter keyboard control when triggered via space key #14067
Conversation
KenanYusuf
commented
Aug 1, 2024
•
KenanYusuf
commented
- Fix custom header filter keyboard control when triggered via space key
- Fixes [data grid] Custom header filter keyboard navigation issues #13734
- I have followed (at least) the PR section of the contributing guide.
| @@ -259,6 +259,7 @@ const GridHeaderFilterCell = React.forwardRef<HTMLDivElement, GridHeaderFilterCe | |||
| if (inputRef.current?.contains?.(event.target as HTMLElement)) { | |||
| inputRef.current.focus(); | |||
| } | |||
| apiRef.current.startHeaderFilterEditMode(colDef.field); | |||
There was a problem hiding this comment.
@MBilalShafi unsure if there will be any side effects to this change, but it made sense to me in theory that when a custom filter is clicked it should be in edit mode
There was a problem hiding this comment.
I think it will change the current behavior of the header filters.
Each header filter cell has two states:
- Focus state: The cell is focused but is not in edit mode. The user can use arrow keys to navigate to other cells.
- Edit state: The cell is focused and is in the edit state. The user can use arrow keys to edit/modify the text input.
How to shift from Focus state to Edit state?
- Start typing something.
- Press enter
How to shift from Edit state to Focus state?
While in Edit state, press enter.
If we make it go to the edit mode on click:
- Keyboard navigation will not work as expected (until an Enter is pressed) which might be annoying.
- Should it behave the same when navigating using arrow keys? IMO it'll be a bad UX for the users who use keyboard navigation.
- Instead of clicking on the cell, the user can directly click on the input to activate the edit mode right away.
Coming to the problem of not activating the Autocomplete field on Start typing, I think that's because the inputRef is not being passed on to the custom component being used. Since the Data Grid internally uses the inputRef to focus the input field when required.
I am not sure if it's the best of the solutions, but it could be partially solved by passing the inputRef to the input component, I tried to do this on top of the original example: https://codesandbox.io/p/sandbox/suspicious-feynman-y3p535
The proper fix would be done with #9243 I guess that would handle selecting multiple values internally.
There was a problem hiding this comment.
I am not sure if it's the best of the solutions, but it could be partially solved by passing the inputRef to the input component, I tried to do this on top of the original example: https://codesandbox.io/p/sandbox/suspicious-feynman-y3p535
This makes sense, and it fixes the user's example. I will go back to them with this fix and revert the change here.
KenanYusuf
added
accessibility
|
Deploy preview: https://deploy-preview-14067--material-ui-x.netlify.app/ |
| @@ -183,7 +183,7 @@ const GridHeaderFilterCell = React.forwardRef<HTMLDivElement, GridHeaderFilterCe | |||
|
|
|||
| const onKeyDown = React.useCallback( | |||
| (event: React.KeyboardEvent) => { | |||
| if (isMenuOpen || isNavigationKey(event.key) || isFilterReadOnly) { | |||
| if (isMenuOpen || (event.key !== ' ' && isNavigationKey(event.key)) || isFilterReadOnly) { | |||
There was a problem hiding this comment.
Space is technically a navigation key, but this is an exception as it can also be used to trigger inputs
There was a problem hiding this comment.
Not sure, I chose not to do this for consistency as it's also not with the normal Grid cells. It was the case for the normal Grid cells too, but we removed it while fixing a bug.
There was a problem hiding this comment.
Ok that's interesting. Here's what I am confused about...
In both of the images below, the cells have been navigated to via keyboard, but the controls within the cells look like they are in different states. The text field looks blurred, and the select looks focused.
Should the select field also be in a blurred state to avoid confusion here?
There was a problem hiding this comment.
I agree that the interaction with the singleSelect column type is not great via keyboard, especially with the customizations. (Select / Autocomplete etc)
We could also rethink how the keyboard navigation works with header filters. We have already discussed this in detail during the feature implementation (point no 5). I have a feeling we have room for improvement here. 🤔
Probably we could focus on the input field by default when the cell is focused, but then how the keyboard navigation will work smoothly is something I'm unsure about.
What do you think?
KenanYusuf
added
the
feature: Filtering on header
packages/x-data-grid-pro/src/components/headerFiltering/GridHeaderFilterCell.tsx
Outdated
Show resolved
Hide resolved
Signed-off-by: Kenan Yusuf <[email protected]>
KenanYusuf
changed the title
|
Closing for now as it won't be worked on in the foreseeable future. Needs more thought. |
