[kbn-grid-layout] Cleanup memoization and styling #210285
Merged
+671
−673
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ea0e84d to
e7ec22e
Compare
|
Pinging @elastic/kibana-presentation (Team:Presentation) |
mbondyra
approved these changes
Feb 10, 2025
💚 Build Succeeded
Metrics [docs]Async chunks
History
cc @Heenawter |
nickpeihl
approved these changes
Feb 10, 2025
|
Starting backport for target branches: 8.x https://github.com/elastic/kibana/actions/runs/13249525893 |
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
Heenawter
added a commit
to Heenawter/kibana
that referenced
this pull request
Feb 10, 2025
## Summary This PR cleans up the `kbn-grid-layout` code in two ways: 1. Rather than memoizing components in their parents, I swapped to using `React.memo` for all components, which accomplishes the same behaviour in a slightly cleaner way. 2. I moved all Emotion style definitions **outside** of the React components so that we no longer have to re-parse the CSS string on every render (see [this comment](elastic/eui#6828 (comment))). ### Checklist - [x] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) (cherry picked from commit 219f31a) # Conflicts: # src/platform/packages/private/kbn-grid-layout/grid/grid_panel/resize_handle.tsx
Heenawter
added a commit
that referenced
this pull request
Feb 11, 2025
…10466) # Backport This will backport the following commits from `main` to `8.x`: - [[kbn-grid-layout] Cleanup memoization and styling (#210285)](#210285) <!--- Backport version: 9.6.4 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Hannah Mudge","email":"Heenawter@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-02-05T22:18:04Z","message":"[Dashboard] Presentation panel refactor (#207275)\n\nCloses https://github.com/elastic/kibana/issues/206686\r\nCloses https://github.com/elastic/kibana/issues/197897\r\nPart of https://github.com/elastic/kibana/issues/207852\r\n\r\n## Summary\r\n\r\nThis PR is a major refactor of the `PresentationPanel` component,\r\nincluding an overhaul of the hover action and panel title components.\r\nSome notable highlights include:\r\n- All styles in the `PresentationPanel` component were moved from SASS\r\nto Emotion\r\n- The over-complicated logic to combine hover actions when the panel\r\nshrinks was removed in favour of CSS, driven by a [container\r\nquery](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_containment/Container_queries)\r\nRemoving the `updateCombineHoverActions` function (which was defined in\r\na React component and not memoized) also made a difference in\r\nperformance when dragging:\r\n \r\n | Before | After |\r\n |--------|--------|\r\n|\r\n\r\n|\r\n\r\n|\r\n \r\n\r\n- The over-complicated logic defined in\r\n`usePresentationPanelTitleClickHandle`, which was meant to ignore the\r\n`onClick` that would trigger after a panel was dragged, was converted to\r\n2 lines of CSS\r\n\r\n### Small usability improvements\r\n\r\nThis PR also includes a few small usability improvements, such as:\r\n\r\n- Ensuring that only the **first** row of hover actions overlaps with\r\nthe Dashboard's sticky top navigation bar, and this only happens when\r\nthe dashboard has no controls. This results in much better behaviour in\r\nmost scenarios:\r\n \r\n | Before | After |\r\n |--------|--------|\r\n| \r\n| \r\n|\r\n\r\n- Adding a small delay for hiding the hover actions on mouse leave,\r\nwhich makes it a lot easier to grab the drag handle:\r\n\r\n | Before | After |\r\n |--------|--------|\r\n| \r\n| \r\n|\r\n\r\n- Preventing the resize handle from overlapping Dashboard's stick top\r\nnavigation:\r\n\r\n | Before | After |\r\n |--------|--------|\r\n| \r\n| \r\n|\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] The PR description includes the appropriate Release Notes section,\r\nand the correct `release_note:*` label is applied per the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"c35698bcf81314f833467fde2d3c14785b83c1ad","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["performance","Team:Presentation","loe:medium","technical debt","release_note:skip","impact:high","backport:version","v9.1.0","v8.19.0"],"title":"[Dashboard] Presentation panel refactor","number":207275,"url":"https://github.com/elastic/kibana/pull/207275","mergeCommit":{"message":"[Dashboard] Presentation panel refactor (#207275)\n\nCloses https://github.com/elastic/kibana/issues/206686\r\nCloses https://github.com/elastic/kibana/issues/197897\r\nPart of https://github.com/elastic/kibana/issues/207852\r\n\r\n## Summary\r\n\r\nThis PR is a major refactor of the `PresentationPanel` component,\r\nincluding an overhaul of the hover action and panel title components.\r\nSome notable highlights include:\r\n- All styles in the `PresentationPanel` component were moved from SASS\r\nto Emotion\r\n- The over-complicated logic to combine hover actions when the panel\r\nshrinks was removed in favour of CSS, driven by a [container\r\nquery](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_containment/Container_queries)\r\nRemoving the `updateCombineHoverActions` function (which was defined in\r\na React component and not memoized) also made a difference in\r\nperformance when dragging:\r\n \r\n | Before | After |\r\n |--------|--------|\r\n|\r\n\r\n|\r\n\r\n|\r\n \r\n\r\n- The over-complicated logic defined in\r\n`usePresentationPanelTitleClickHandle`, which was meant to ignore the\r\n`onClick` that would trigger after a panel was dragged, was converted to\r\n2 lines of CSS\r\n\r\n### Small usability improvements\r\n\r\nThis PR also includes a few small usability improvements, such as:\r\n\r\n- Ensuring that only the **first** row of hover actions overlaps with\r\nthe Dashboard's sticky top navigation bar, and this only happens when\r\nthe dashboard has no controls. This results in much better behaviour in\r\nmost scenarios:\r\n \r\n | Before | After |\r\n |--------|--------|\r\n| \r\n| \r\n|\r\n\r\n- Adding a small delay for hiding the hover actions on mouse leave,\r\nwhich makes it a lot easier to grab the drag handle:\r\n\r\n | Before | After |\r\n |--------|--------|\r\n| \r\n| \r\n|\r\n\r\n- Preventing the resize handle from overlapping Dashboard's stick top\r\nnavigation:\r\n\r\n | Before | After |\r\n |--------|--------|\r\n| \r\n| \r\n|\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] The PR description includes the appropriate Release Notes section,\r\nand the correct `release_note:*` label is applied per the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"c35698bcf81314f833467fde2d3c14785b83c1ad"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/207275","number":207275,"mergeCommit":{"message":"[Dashboard] Presentation panel refactor (#207275)\n\nCloses https://github.com/elastic/kibana/issues/206686\r\nCloses https://github.com/elastic/kibana/issues/197897\r\nPart of https://github.com/elastic/kibana/issues/207852\r\n\r\n## Summary\r\n\r\nThis PR is a major refactor of the `PresentationPanel` component,\r\nincluding an overhaul of the hover action and panel title components.\r\nSome notable highlights include:\r\n- All styles in the `PresentationPanel` component were moved from SASS\r\nto Emotion\r\n- The over-complicated logic to combine hover actions when the panel\r\nshrinks was removed in favour of CSS, driven by a [container\r\nquery](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_containment/Container_queries)\r\nRemoving the `updateCombineHoverActions` function (which was defined in\r\na React component and not memoized) also made a difference in\r\nperformance when dragging:\r\n \r\n | Before | After |\r\n |--------|--------|\r\n|\r\n\r\n|\r\n\r\n|\r\n \r\n\r\n- The over-complicated logic defined in\r\n`usePresentationPanelTitleClickHandle`, which was meant to ignore the\r\n`onClick` that would trigger after a panel was dragged, was converted to\r\n2 lines of CSS\r\n\r\n### Small usability improvements\r\n\r\nThis PR also includes a few small usability improvements, such as:\r\n\r\n- Ensuring that only the **first** row of hover actions overlaps with\r\nthe Dashboard's sticky top navigation bar, and this only happens when\r\nthe dashboard has no controls. This results in much better behaviour in\r\nmost scenarios:\r\n \r\n | Before | After |\r\n |--------|--------|\r\n| \r\n| \r\n|\r\n\r\n- Adding a small delay for hiding the hover actions on mouse leave,\r\nwhich makes it a lot easier to grab the drag handle:\r\n\r\n | Before | After |\r\n |--------|--------|\r\n| \r\n| \r\n|\r\n\r\n- Preventing the resize handle from overlapping Dashboard's stick top\r\nnavigation:\r\n\r\n | Before | After |\r\n |--------|--------|\r\n| \r\n| \r\n|\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n- [x] The PR description includes the appropriate Release Notes section,\r\nand the correct `release_note:*` label is applied per the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"c35698bcf81314f833467fde2d3c14785b83c1ad"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR cleans up the
kbn-grid-layoutcode in two ways:React.memofor all components, which accomplishes the same behaviour in a slightly cleaner way.Checklist
release_note:*label is applied per the guidelines