Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[EuiDataGrid] Keep focus on grid if column gets removed #4923

Closed
Tracked by #85965
flash1293 opened this issue Jun 28, 2021 · 0 comments · Fixed by #5530
Closed
Tracked by #85965

[EuiDataGrid] Keep focus on grid if column gets removed #4923

flash1293 opened this issue Jun 28, 2021 · 0 comments · Fixed by #5530
Labels
accessibility bug data grid

Comments

@flash1293
Copy link
Contributor

If a currently focused column gets removed from data grid, the focus isn't captured anymore and returns to the document body.

Data grid should recognize and handle this case by moving the focus to the grid root element.
@flash1293 flash1293 mentioned this issue Jun 28, 2021
Closed
@myasonik myasonik added the bug label Jun 28, 2021
@timroes timroes mentioned this issue Aug 18, 2021
Closed
41 tasks
@cee-chen cee-chen mentioned this issue Jan 6, 2022
Closed
cee-chen added a commit to cee-chen/eui that referenced this issue Jan 11, 2022
@cee-chen
[elastic#4923] Fix focus returning to document body when hiding a col…
d29d938 
…umn via header
@cee-chen cee-chen mentioned this issue Jan 11, 2022
Merged
5 tasks
cee-chen pushed a commit that referenced this issue Jan 12, 2022
[EuiDataGrid] Fix multiple grid focus restoration issues (#5530)
8cb2c3f 
* [#5517] Add `isFocusedCellInView` state to track visibility

- and use this to determine whether or not the entire grid should have a tabindex/be focusable, rather than a simple 'hasFocus' check which doesn't account for virtualization

- Add new mockFocusContext for easier testing for various tests that need to set a focus context

- EuiDataGridCell
  - DRY out an `this.isFocusedCell` method
  - Call `setIsFocusedCellInView` on mount and on unmount (`setFocusedCell` handles setting true/false on new cell focus)
  - Write new unit tests for componentWillUnmount
  - Clean up unnecessary uncovered function fallback for this.unsubscribeCell - we're already checking for a falsy value before calling it

* [#5517] Add `onItemsRendered` ref to store currently visible/virtualized refs

- see https://react-window.vercel.app/#/api/FixedSizeGrid for `onItemsRendered` API
- this struck me as the quickest/easiest way to determine which virtualized cells are in view
- I opted to save this as a ref instead of as state as I didn't see the need to cause a rerender

- `focusFirstVisibleInteractiveCell` was split out to its own fn as it will shortly be used elsewhere to fix another focus bug in the grid

* [#4923] Fix focus returning to document body when hiding a column via header

* [#5476] Fix keyboard navigation after hiding a column via header actions

* Add changelog entry

* [PR feedback] Add explanatory comment in unit test

* [PR feedback] Account for empty grids

- gridItemsRendered will be falsey if rowCount is 0 and will cause an error on tab, so we should opt to simply leave focus on the grid body wrapper

* [bug] Handle arrow keydown behavior on sticky header when row 0 is not in view
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
accessibility bug data grid
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[EuiDataGrid] Fix multiple grid focus restoration issues cee-chen/eui
2 participants
@flash1293 @myasonik