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.

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

[EuiDataGrid][KEYBOARD]: Reordering columns causes a jump when navigating by arrow keys #5476

Closed
Tracked by #85965
1Copenut opened this issue Dec 15, 2021 · 0 comments · Fixed by #5530
Closed
Tracked by #85965

Comments

@1Copenut
Copy link
Contributor

While testing the EuiDataGrid component for Kibana Discover, I noticed the column reordering behavior "jumps" from a column's previous place instead of its current one when I navigate to another cell with arrow keys.

Movie below shows navigation using keyboard only, and highlights the column jump at the 0:11 mark.


data-grid-column-order.mov
@1Copenut 1Copenut changed the title [EuiDataGrid] Reordering columns causes a jump when navigating by arrow keys [EuiDataGrid][KEYBOARD]: Reordering columns causes a jump when navigating by arrow keys Dec 15, 2021
cee-chen added a commit to cee-chen/eui that referenced this issue Jan 11, 2022
cee-chen pushed a commit that referenced this issue Jan 12, 2022
* [#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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant