Skip to content

Prevent datagrid cells from queuing up focus ownership#4983

Merged
chandlerprall merged 2 commits intoelastic:masterfrom
chandlerprall:bug/4975-datagrid-cell-focus-queuing
Jul 28, 2021
Merged

Prevent datagrid cells from queuing up focus ownership#4983
chandlerprall merged 2 commits intoelastic:masterfrom
chandlerprall:bug/4975-datagrid-cell-focus-queuing

Conversation

@chandlerprall
Copy link
Copy Markdown
Contributor

@chandlerprall chandlerprall commented Jul 27, 2021

Summary

Fixes #4975

A few things came together to create this bug:

  • EuiDataGrid owns which cell is focused
  • EuiDataGridCell subscribes to notifications about its focus state and responds accordingly by accepting focus
  • EuiDataGridCell also reacts to it receiving focus by e.g. a user clicking on it, it tells the parent grid about the updated focus state in a setTimeout which is meant to allow any focus traps to deactivate before the cell re-claims focus
  • If multiple cells receive focus within that same setTimeout delay, they each queued up the focus claim
  • As the focus queue was processed, each cell noticed it was no longer focused and would re-queue its claim

This fix avoids any queuing by merging the timeouts into one.

Checklist

- [ ] Check against all themes for compatibility in both light and dark modes
- [ ] Checked in mobile

  • Checked in Chrome, Safari, Edge, and Firefox
    - [ ] Props have proper autodocs and playground toggles
    - [ ] Added documentation
    - [ ] Checked Code Sandbox works for the any docs examples
    - [ ] Added or updated jest tests
  • Checked for breaking changes and labeled appropriately
    - [ ] Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

@chandlerprall chandlerprall requested a review from thompsongl July 27, 2021 21:19
@kibanamachine
Copy link
Copy Markdown

Preview documentation changes for this PR: https://eui.elastic.co/pr_4983/

Copy link
Copy Markdown
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a functional test of this and it solves the problems @andrew-goldstein and I noticed.

Copy link
Copy Markdown
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; tested with docs preview.
EUI cheat codes restored 👾

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[EuiDataGrid] Quickly pressing the arrow keys triggers an infinite loop

4 participants