github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-24 09:48:58 -04:00
Code Issues Projects Releases Packages Wiki Activity

[8.18] [kbn-grid-layout] [Dashboard] Fix memoization of onLayoutChange callback (#217235) (#217573)

Browse source 
# Backport

This will backport the following commits from `main` to `8.18`:
- [[kbn-grid-layout] [Dashboard] Fix memoization of `onLayoutChange`
callback (#217235)](https://github.com/elastic/kibana/pull/217235)

<!--- Backport version: 9.6.6 -->

### 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-04-04T20:53:17Z","message":"[kbn-grid-layout]
[Dashboard] Fix memoization of `onLayoutChange` callback
(#217235)\n\nCloses
https://github.com/elastic/kibana/issues/217117\n\n## Summary\n\nIn the
dashboard grid component, we memoize the `onLayoutChange`\ncallback with
`viewMode` as a
dependency:\n\n\n64142047fb/src/platform/plugins/shared/dashboard/public/dashboard_renderer/grid/dashboard_grid.tsx (L80-L105)\n\nHowever,
on the `kbn-grid-layout` side, the subscription responsible for\ncalling
`onLayoutChange` was set up in a `useEffect` that did **not**\nhave the
`onLayoutChange` prop as a dependency - so even though the prop\nwas
changing, the subscription continued to use the original value.\n\nThat
means that if the dashboard starts in view mode, then
the\n`kbn-grid-layout` layout change subscription will always be calling
the\nversion of `onLayoutChange` where it returns early; and likewise
when\nyou start the dashboard in edit mode. By adding `onLayoutChange`
as a\ndependency to the `useEffect` that sets up the subscriptions, this
no\nlonger happens.\n\n###
Before\n\n\n\nhttps://github.com/user-attachments/assets/5df796f5-929d-4522-b438-64cc49292ed6\n\n###
After\n\n\n\nhttps://github.com/user-attachments/assets/286dd459-b429-40fb-8cb7-661b3f870214\n\n\n\n###
Checklist\n\n- [x] The PR description includes the appropriate Release
Notes section,\nand the correct `release_note:*` label is applied per
the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"8382475eb5f68b804d6c7f1b59b50b4ff956c59e","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["regression","release_note:fix","Team:Presentation","loe:small","impact:high","backport:version","v9.1.0","v8.19.0","v8.18.1","v9.0.1"],"title":"[kbn-grid-layout]
[Dashboard] Fix memoization of `onLayoutChange`
callback","number":217235,"url":"https://github.com/elastic/kibana/pull/217235","mergeCommit":{"message":"[kbn-grid-layout]
[Dashboard] Fix memoization of `onLayoutChange` callback
(#217235)\n\nCloses
https://github.com/elastic/kibana/issues/217117\n\n## Summary\n\nIn the
dashboard grid component, we memoize the `onLayoutChange`\ncallback with
`viewMode` as a
dependency:\n\n\n64142047fb/src/platform/plugins/shared/dashboard/public/dashboard_renderer/grid/dashboard_grid.tsx (L80-L105)\n\nHowever,
on the `kbn-grid-layout` side, the subscription responsible for\ncalling
`onLayoutChange` was set up in a `useEffect` that did **not**\nhave the
`onLayoutChange` prop as a dependency - so even though the prop\nwas
changing, the subscription continued to use the original value.\n\nThat
means that if the dashboard starts in view mode, then
the\n`kbn-grid-layout` layout change subscription will always be calling
the\nversion of `onLayoutChange` where it returns early; and likewise
when\nyou start the dashboard in edit mode. By adding `onLayoutChange`
as a\ndependency to the `useEffect` that sets up the subscriptions, this
no\nlonger happens.\n\n###
Before\n\n\n\nhttps://github.com/user-attachments/assets/5df796f5-929d-4522-b438-64cc49292ed6\n\n###
After\n\n\n\nhttps://github.com/user-attachments/assets/286dd459-b429-40fb-8cb7-661b3f870214\n\n\n\n###
Checklist\n\n- [x] The PR description includes the appropriate Release
Notes section,\nand the correct `release_note:*` label is applied per
the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"8382475eb5f68b804d6c7f1b59b50b4ff956c59e"}},"sourceBranch":"main","suggestedTargetBranches":["8.18"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/217235","number":217235,"mergeCommit":{"message":"[kbn-grid-layout]
[Dashboard] Fix memoization of `onLayoutChange` callback
(#217235)\n\nCloses
https://github.com/elastic/kibana/issues/217117\n\n## Summary\n\nIn the
dashboard grid component, we memoize the `onLayoutChange`\ncallback with
`viewMode` as a
dependency:\n\n\n64142047fb/src/platform/plugins/shared/dashboard/public/dashboard_renderer/grid/dashboard_grid.tsx (L80-L105)\n\nHowever,
on the `kbn-grid-layout` side, the subscription responsible for\ncalling
`onLayoutChange` was set up in a `useEffect` that did **not**\nhave the
`onLayoutChange` prop as a dependency - so even though the prop\nwas
changing, the subscription continued to use the original value.\n\nThat
means that if the dashboard starts in view mode, then
the\n`kbn-grid-layout` layout change subscription will always be calling
the\nversion of `onLayoutChange` where it returns early; and likewise
when\nyou start the dashboard in edit mode. By adding `onLayoutChange`
as a\ndependency to the `useEffect` that sets up the subscriptions, this
no\nlonger happens.\n\n###
Before\n\n\n\nhttps://github.com/user-attachments/assets/5df796f5-929d-4522-b438-64cc49292ed6\n\n###
After\n\n\n\nhttps://github.com/user-attachments/assets/286dd459-b429-40fb-8cb7-661b3f870214\n\n\n\n###
Checklist\n\n- [x] The PR description includes the appropriate Release
Notes section,\nand the correct `release_note:*` label is applied per
the\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"8382475eb5f68b804d6c7f1b59b50b4ff956c59e"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/217259","number":217259,"state":"MERGED","mergeCommit":{"sha":"29033d81aa7cf3b63bed0a76fbb7b196eab62329","message":"[8.x]
[kbn-grid-layout] [Dashboard] Fix memoization of `onLayoutChange`
callback (#217235) (#217259)\n\n# Backport\n\nThis will backport the
following commits from `main` to `8.x`:\n- [[kbn-grid-layout]
[Dashboard] Fix memoization of `onLayoutChange`\ncallback
(#217235)](https://github.com/elastic/kibana/pull/217235)\n\n\n\n###
Questions ?\nPlease refer to the [Backport
tool\ndocumentation](https://github.com/sorenlouv/backport)\n\n\n\nCo-authored-by:
Hannah Mudge
<Heenawter@users.noreply.github.com>"}},{"branch":"8.18","label":"v8.18.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"9.0","label":"v9.0.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/217260","number":217260,"state":"OPEN"}]}]
BACKPORT-->

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Hannah Mudge 2025-04-09 02:53:17 -06:00 • committed by GitHub
parent 09d4d4f3f6
commit 3654301127
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 18 additions and 12 deletions
Download patch file Download diff file Expand all files Collapse all files

30
src/platform/packages/private/kbn-grid-layout/grid/grid_layout.tsx
View file

@ -77,6 +77,24 @@ export const GridLayout = ({
/**
* Set up subscriptions
*/
useEffect(() => {
/**
* This subscription calls the passed `onLayoutChange` callback when the layout changes
*/
const onLayoutChangeSubscription = gridLayoutStateManager.gridLayout$
.pipe(pairwise())
.subscribe(([layoutBefore, layoutAfter]) => {
if (!isLayoutEqual(layoutBefore, layoutAfter)) {
onLayoutChange(layoutAfter);
}
});
return () => {
onLayoutChangeSubscription.unsubscribe();
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [onLayoutChange]);
useEffect(() => {
/**
* The only thing that should cause the entire layout to re-render is adding a new row;
@ -92,17 +110,6 @@ export const GridLayout = ({
setRowCount(newRowCount);
});
/**
* This subscription calls the passed `onLayoutChange` callback when the layout changes
*/
const onLayoutChangeSubscription = gridLayoutStateManager.gridLayout$
.pipe(pairwise())
.subscribe(([layoutBefore, layoutAfter]) => {
if (!isLayoutEqual(layoutBefore, layoutAfter)) {
onLayoutChange(layoutAfter);
}
});
/**
* This subscription adds and/or removes the necessary class names related to styling for
* mobile view and a static (non-interactable) grid layout
@ -128,7 +135,6 @@ export const GridLayout = ({
return () => {
rowCountSubscription.unsubscribe();
onLayoutChangeSubscription.unsubscribe();
gridLayoutClassSubscription.unsubscribe();
};
// eslint-disable-next-line react-hooks/exhaustive-deps