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] Fix cell performance #7465

Merged
merged 5 commits into from
Jan 17, 2024
Merged

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented Jan 17, 2024

Summary

Fixes datagrid performance issues caused by #7448

This is particularly noticeable when comparing toggling between fullscreen and non-fullscreen mode:

QA

General checklist

  • Browser QA
    - [ ] Checked in both light and dark modes
    - [ ] Checked in mobile
    - [ ] Checked for accessibility including keyboard-only and screenreader modes
    • Checked in Chrome, Safari, Edge, and Firefox
  • Docs site QA - N/A
  • Code quality checklist
  • Release checklist
    • A changelog entry exists and is marked appropriately.
      - [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)
  • Designer checklist - N/A

- memoize all fns to class methods

- do not define jsx as const, this causes react to redefine a new component each rerender
…ed or focus cells

- appears to be significantly more performant
@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

Copy link
Contributor

@kqualters-elastic kqualters-elastic left a comment

Choose a reason for hiding this comment

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

Much better 👍 similar patterns are found throughout the library, cleaning them up will have 0 user facing changes and likely small but potentially pretty large performance improvement everywhere, thank you!

@cee-chen cee-chen marked this pull request as ready for review January 17, 2024 15:33
@cee-chen cee-chen requested a review from a team as a code owner January 17, 2024 15:33
Copy link
Member

@JasonStoltz JasonStoltz left a comment

Choose a reason for hiding this comment

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

LGTM

@cee-chen
Copy link
Member Author

Thanks a million y'all!

@cee-chen cee-chen merged commit 7764195 into elastic:main Jan 17, 2024
8 checks passed
@cee-chen cee-chen deleted the datagrid-cell-perf branch January 17, 2024 16:01
cee-chen added a commit to elastic/kibana that referenced this pull request Jan 24, 2024
`v92.0.0-backport.0`⏩ `v92.1.1`

---

## [`v92.1.1`](https://github.com/elastic/eui/releases/v92.1.1)

**Bug fixes**

- Minor `EuiDataGrid` cell performance fixes
([#7465](elastic/eui#7465))

## [`v92.1.0`](https://github.com/elastic/eui/releases/v92.1.0)

- Updated `EuiResizableButton` to allow customizing the `indicator`
style with either `handle` (default) or `border`
([#7455](elastic/eui#7455))
- Enhanced `EuiResizableContainer` to preserve the drag/resize event
when the user's mouse leaves the parent container and re-enters
([#7456](elastic/eui#7456))

**Bug fixes**

- Fixed an `EuiTreeView` JSX Typescript error
([#7452](elastic/eui#7452))
- Fixed a color console warning being generated by disabled `EuiStep`s
([#7454](elastic/eui#7454))

**Accessibility**

- `EuiDataGrid`'s keyboard/screenreader experience has been tweaked to
be more consistent for varying complex data:
([#7448](elastic/eui#7448))
- Headers are now always navigable by arrow key, regardless of whether
the header cells contain interactive content
- Non-expandable cells containing any amount of interactive content now
must be entered via Enter or F2 keypress
  - Expandable cells continue to be toggled via Enter or F2 keypress
- `EuiDataGrid` now provides a direct screen reader hint for Enter key
behavior for expandable & interactive cells
([#7448](elastic/eui#7448))
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
`v92.0.0-backport.0`⏩ `v92.1.1`

---

## [`v92.1.1`](https://github.com/elastic/eui/releases/v92.1.1)

**Bug fixes**

- Minor `EuiDataGrid` cell performance fixes
([elastic#7465](elastic/eui#7465))

## [`v92.1.0`](https://github.com/elastic/eui/releases/v92.1.0)

- Updated `EuiResizableButton` to allow customizing the `indicator`
style with either `handle` (default) or `border`
([elastic#7455](elastic/eui#7455))
- Enhanced `EuiResizableContainer` to preserve the drag/resize event
when the user's mouse leaves the parent container and re-enters
([elastic#7456](elastic/eui#7456))

**Bug fixes**

- Fixed an `EuiTreeView` JSX Typescript error
([elastic#7452](elastic/eui#7452))
- Fixed a color console warning being generated by disabled `EuiStep`s
([elastic#7454](elastic/eui#7454))

**Accessibility**

- `EuiDataGrid`'s keyboard/screenreader experience has been tweaked to
be more consistent for varying complex data:
([elastic#7448](elastic/eui#7448))
- Headers are now always navigable by arrow key, regardless of whether
the header cells contain interactive content
- Non-expandable cells containing any amount of interactive content now
must be entered via Enter or F2 keypress
  - Expandable cells continue to be toggled via Enter or F2 keypress
- `EuiDataGrid` now provides a direct screen reader hint for Enter key
behavior for expandable & interactive cells
([elastic#7448](elastic/eui#7448))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants