[Dashboard] Presentation panel refactor#207275
Merged
Heenawter merged 45 commits intoelastic:mainfrom Feb 5, 2025
Merged
Conversation
3 tasks
…ctions-performance_2025-01-16
andreadelrio
approved these changes
Feb 5, 2025
Contributor
andreadelrio
left a comment
There was a problem hiding this comment.
Design changes LGTM. Amazing cleanup and improvements!
Contributor
|
Starting backport for target branches: 8.x |
Contributor
|
Starting backport for target branches: 8.x |
Contributor
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
1 similar comment
Contributor
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation |
Heenawter
added a commit
that referenced
this pull request
Feb 6, 2025
…#209937) ## Summary Since #207275 is too large to backport to `9.0`/`8.18` and will only be in `9.1`/`8.19`, I wanted to at least backport **just** the z-index fix for the resize handler as described [here](https://github.com/elastic/kibana/pull/207275/files#r1931305375) . Unfortunately, the z-index used in that PR (`euiTheme.levels.maskBelowHeader`) only works thanks to other hover-action style changes, so I've had to set a hardcoded z-index (`2000`) in this PR instead. This acts as a "quick fix" for a pretty annoying bug while avoiding the risk of backporting the entire presentation panel refactor PR to `9.0`/`8.18` | Before | After | |--------|--------| |  |  |
Contributor
Author
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
Heenawter
added a commit
to Heenawter/kibana
that referenced
this pull request
Feb 6, 2025
Closes elastic#206686 Closes elastic#197897 Part of elastic#207852 ## Summary This PR is a major refactor of the `PresentationPanel` component, including an overhaul of the hover action and panel title components. Some notable highlights include: - All styles in the `PresentationPanel` component were moved from SASS to Emotion - The over-complicated logic to combine hover actions when the panel shrinks was removed in favour of CSS, driven by a [container query](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_containment/Container_queries) Removing the `updateCombineHoverActions` function (which was defined in a React component and not memoized) also made a difference in performance when dragging: | Before | After | |--------|--------| |  |  | - The over-complicated logic defined in `usePresentationPanelTitleClickHandle`, which was meant to ignore the `onClick` that would trigger after a panel was dragged, was converted to 2 lines of CSS ### Small usability improvements This PR also includes a few small usability improvements, such as: - Ensuring that only the **first** row of hover actions overlaps with the Dashboard's sticky top navigation bar, and this only happens when the dashboard has no controls. This results in much better behaviour in most scenarios: | Before | After | |--------|--------| |  |  | - Adding a small delay for hiding the hover actions on mouse leave, which makes it a lot easier to grab the drag handle: | Before | After | |--------|--------| |  |  | - Preventing the resize handle from overlapping Dashboard's stick top navigation: | Before | After | |--------|--------| |  |  | ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [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) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit c35698b) # Conflicts: # src/platform/plugins/private/presentation_panel/public/panel_component/_presentation_panel.scss # src/platform/plugins/shared/dashboard/public/dashboard_container/component/grid/use_layout_styles.tsx
Heenawter
added a commit
to Heenawter/kibana
that referenced
this pull request
Feb 6, 2025
…elastic#209937) ## Summary Since elastic#207275 is too large to backport to `9.0`/`8.18` and will only be in `9.1`/`8.19`, I wanted to at least backport **just** the z-index fix for the resize handler as described [here](https://github.com/elastic/kibana/pull/207275/files#r1931305375) . Unfortunately, the z-index used in that PR (`euiTheme.levels.maskBelowHeader`) only works thanks to other hover-action style changes, so I've had to set a hardcoded z-index (`2000`) in this PR instead. This acts as a "quick fix" for a pretty annoying bug while avoiding the risk of backporting the entire presentation panel refactor PR to `9.0`/`8.18` | Before | After | |--------|--------| |  |  | (cherry picked from commit 7ebe1bf)
Heenawter
added a commit
that referenced
this pull request
Feb 6, 2025
… header (#209937) (#210031) # Backport This will backport the following commits from `9.0` to `8.18`: - [[9.0] [Dashboard] Ensure resize handle does not overlap sticky header (#209937)](#209937) <!--- 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-06T15:03:28Z","message":"[9.0] [Dashboard] Ensure resize handle does not overlap sticky header (#209937)\n\n## Summary\r\n\r\nSince #207275 is too large to\r\nbackport to `9.0`/`8.18` and will only be in `9.1`/`8.19`, I wanted to\r\nat least backport **just** the z-index fix for the resize handler as\r\ndescribed\r\n[here](https://github.com/elastic/kibana/pull/207275/files#r1931305375)\r\n. Unfortunately, the z-index used in that PR\r\n(`euiTheme.levels.maskBelowHeader`) only works thanks to other\r\nhover-action style changes, so I've had to set a hardcoded z-index\r\n(`2000`) in this PR instead. This acts as a \"quick fix\" for a pretty\r\nannoying bug while avoiding the risk of backporting the entire\r\npresentation panel refactor PR to `9.0`/`8.18`\r\n\r\n\r\n| Before | After |\r\n|--------|--------|\r\n| \r\n|\r\n\r\n|","sha":"7ebe1bf8d020e5b732b22aa2a23adaae8bb5bb48","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Presentation","loe:small","release_note:skip","impact:low","backport:version","v8.18.0"],"title":"[9.0] [Dashboard] Ensure resize handle does not overlap sticky header","number":209937,"url":"https://github.com/elastic/kibana/pull/209937","mergeCommit":{"message":"[9.0] [Dashboard] Ensure resize handle does not overlap sticky header (#209937)\n\n## Summary\r\n\r\nSince #207275 is too large to\r\nbackport to `9.0`/`8.18` and will only be in `9.1`/`8.19`, I wanted to\r\nat least backport **just** the z-index fix for the resize handler as\r\ndescribed\r\n[here](https://github.com/elastic/kibana/pull/207275/files#r1931305375)\r\n. Unfortunately, the z-index used in that PR\r\n(`euiTheme.levels.maskBelowHeader`) only works thanks to other\r\nhover-action style changes, so I've had to set a hardcoded z-index\r\n(`2000`) in this PR instead. This acts as a \"quick fix\" for a pretty\r\nannoying bug while avoiding the risk of backporting the entire\r\npresentation panel refactor PR to `9.0`/`8.18`\r\n\r\n\r\n| Before | After |\r\n|--------|--------|\r\n| \r\n|\r\n\r\n|","sha":"7ebe1bf8d020e5b732b22aa2a23adaae8bb5bb48"}},"sourceBranch":"9.0","suggestedTargetBranches":["8.18"],"targetPullRequestStates":[{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
Heenawter
added a commit
that referenced
this pull request
Feb 6, 2025
# Backport This will backport the following commits from `main` to `8.x`: - [[Dashboard] Presentation panel refactor (#207275)](#207275) <!--- 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-->
drewdaemon
pushed a commit
to drewdaemon/kibana
that referenced
this pull request
Feb 6, 2025
Closes elastic#206686 Closes elastic#197897 Part of elastic#207852 ## Summary This PR is a major refactor of the `PresentationPanel` component, including an overhaul of the hover action and panel title components. Some notable highlights include: - All styles in the `PresentationPanel` component were moved from SASS to Emotion - The over-complicated logic to combine hover actions when the panel shrinks was removed in favour of CSS, driven by a [container query](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_containment/Container_queries) Removing the `updateCombineHoverActions` function (which was defined in a React component and not memoized) also made a difference in performance when dragging: | Before | After | |--------|--------| |  |  | - The over-complicated logic defined in `usePresentationPanelTitleClickHandle`, which was meant to ignore the `onClick` that would trigger after a panel was dragged, was converted to 2 lines of CSS ### Small usability improvements This PR also includes a few small usability improvements, such as: - Ensuring that only the **first** row of hover actions overlaps with the Dashboard's sticky top navigation bar, and this only happens when the dashboard has no controls. This results in much better behaviour in most scenarios: | Before | After | |--------|--------| |  |  | - Adding a small delay for hiding the hover actions on mouse leave, which makes it a lot easier to grab the drag handle: | Before | After | |--------|--------| |  |  | - Preventing the resize handle from overlapping Dashboard's stick top navigation: | Before | After | |--------|--------| |  |  | ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [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) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
2 tasks
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-->
Heenawter
added a commit
that referenced
this pull request
Feb 11, 2025
## Summary When #207275 merged, some Security Cypress tests started failing because the embeddables were no longer taking up any width in their containers. This was caused by us switching over to using the CSS container type `inline-size` on the hover anchor wrapper, which makes it so that it can no longer be sized to its contents (see [this comment](https://stackoverflow.com/a/73980194/28754956) for a really good explanation). Instead, the width of the `PresentationPanel` needs to be set **by the parent** - so, by applying the `min-width` to the Metric wrapper rather than the Lens embeddable, the metric now takes up the expected width: | Before | After | |--------|--------| |  |  | When doing this work, I noticed that, because we added a delay to hiding the hover actions, there was a slightly jarring transition when hovering over panels without `border` enabled. So, I fixed this by adding a transition to the border on the panel as well, so that it matches the animation on the hover actions: | Before | After | |--------|--------| |  |  | ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [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)
Heenawter
added a commit
to Heenawter/kibana
that referenced
this pull request
Feb 11, 2025
## Summary When elastic#207275 merged, some Security Cypress tests started failing because the embeddables were no longer taking up any width in their containers. This was caused by us switching over to using the CSS container type `inline-size` on the hover anchor wrapper, which makes it so that it can no longer be sized to its contents (see [this comment](https://stackoverflow.com/a/73980194/28754956) for a really good explanation). Instead, the width of the `PresentationPanel` needs to be set **by the parent** - so, by applying the `min-width` to the Metric wrapper rather than the Lens embeddable, the metric now takes up the expected width: | Before | After | |--------|--------| |  |  | When doing this work, I noticed that, because we added a delay to hiding the hover actions, there was a slightly jarring transition when hovering over panels without `border` enabled. So, I fixed this by adding a transition to the border on the panel as well, so that it matches the animation on the hover actions: | Before | After | |--------|--------| |  |  | ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [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 3716441) # Conflicts: # x-pack/test/security_solution_cypress/cypress/e2e/explore/inspect/inspect_button.cy.ts
Heenawter
added a commit
that referenced
this pull request
Feb 12, 2025
# Backport This will backport the following commits from `main` to `8.x`: - [[Embeddable] Fix presentation panel styles (#210113)](#210113) <!--- 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-11T18:11:18Z","message":"[Embeddable] Fix presentation panel styles (#210113)\n\n## Summary\r\n\r\nWhen #207275 merged, some Security\r\nCypress tests started failing because the embeddables were no longer\r\ntaking up any width in their containers. This was caused by us switching\r\nover to using the CSS container type `inline-size` on the hover anchor\r\nwrapper, which makes it so that it can no longer be sized to its\r\ncontents (see [this\r\ncomment](https://stackoverflow.com/a/73980194/28754956) for a really\r\ngood explanation). Instead, the width of the `PresentationPanel` needs\r\nto be set **by the parent** - so, by applying the `min-width` to the\r\nMetric wrapper rather than the Lens embeddable, the metric now takes up\r\nthe expected width:\r\n\r\n| Before | After |\r\n|--------|--------|\r\n|\r\n\r\n|\r\n\r\n|\r\n\r\nWhen doing this work, I noticed that, because we added a delay to hiding\r\nthe hover actions, there was a slightly jarring transition when hovering\r\nover panels without `border` enabled. So, I fixed this by adding a\r\ntransition to the border on the panel as well, so that it matches the\r\nanimation on the hover actions:\r\n\r\n| Before | After |\r\n|--------|--------|\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)","sha":"3716441c38459d911145c40fea4322ff53927ab4","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:medium","Feature:Embeddables","backport:version","v9.1.0","v8.19.0"],"title":"[Embeddable] Fix presentation panel styles","number":210113,"url":"https://github.com/elastic/kibana/pull/210113","mergeCommit":{"message":"[Embeddable] Fix presentation panel styles (#210113)\n\n## Summary\r\n\r\nWhen #207275 merged, some Security\r\nCypress tests started failing because the embeddables were no longer\r\ntaking up any width in their containers. This was caused by us switching\r\nover to using the CSS container type `inline-size` on the hover anchor\r\nwrapper, which makes it so that it can no longer be sized to its\r\ncontents (see [this\r\ncomment](https://stackoverflow.com/a/73980194/28754956) for a really\r\ngood explanation). Instead, the width of the `PresentationPanel` needs\r\nto be set **by the parent** - so, by applying the `min-width` to the\r\nMetric wrapper rather than the Lens embeddable, the metric now takes up\r\nthe expected width:\r\n\r\n| Before | After |\r\n|--------|--------|\r\n|\r\n\r\n|\r\n\r\n|\r\n\r\nWhen doing this work, I noticed that, because we added a delay to hiding\r\nthe hover actions, there was a slightly jarring transition when hovering\r\nover panels without `border` enabled. So, I fixed this by adding a\r\ntransition to the border on the panel as well, so that it matches the\r\nanimation on the hover actions:\r\n\r\n| Before | After |\r\n|--------|--------|\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)","sha":"3716441c38459d911145c40fea4322ff53927ab4"}},"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/210113","number":210113,"mergeCommit":{"message":"[Embeddable] Fix presentation panel styles (#210113)\n\n## Summary\r\n\r\nWhen #207275 merged, some Security\r\nCypress tests started failing because the embeddables were no longer\r\ntaking up any width in their containers. This was caused by us switching\r\nover to using the CSS container type `inline-size` on the hover anchor\r\nwrapper, which makes it so that it can no longer be sized to its\r\ncontents (see [this\r\ncomment](https://stackoverflow.com/a/73980194/28754956) for a really\r\ngood explanation). Instead, the width of the `PresentationPanel` needs\r\nto be set **by the parent** - so, by applying the `min-width` to the\r\nMetric wrapper rather than the Lens embeddable, the metric now takes up\r\nthe expected width:\r\n\r\n| Before | After |\r\n|--------|--------|\r\n|\r\n\r\n|\r\n\r\n|\r\n\r\nWhen doing this work, I noticed that, because we added a delay to hiding\r\nthe hover actions, there was a slightly jarring transition when hovering\r\nover panels without `border` enabled. So, I fixed this by adding a\r\ntransition to the border on the panel as well, so that it matches the\r\nanimation on the hover actions:\r\n\r\n| Before | After |\r\n|--------|--------|\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)","sha":"3716441c38459d911145c40fea4322ff53927ab4"}},{"branch":"8.x","label":"v8.19.0","branchLabelMappingKey":"^v8.19.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Closes #206686
Closes #197897
Part of #207852
Summary
This PR is a major refactor of the
PresentationPanelcomponent, including an overhaul of the hover action and panel title components. Some notable highlights include:All styles in the
PresentationPanelcomponent were moved from SASS to EmotionThe over-complicated logic to combine hover actions when the panel shrinks was removed in favour of CSS, driven by a container query Removing the
updateCombineHoverActionsfunction (which was defined in a React component and not memoized) also made a difference in performance when dragging:The over-complicated logic defined in
usePresentationPanelTitleClickHandle, which was meant to ignore theonClickthat would trigger after a panel was dragged, was converted to 2 lines of CSSSmall usability improvements
This PR also includes a few small usability improvements, such as:
Ensuring that only the first row of hover actions overlaps with the Dashboard's sticky top navigation bar, and this only happens when the dashboard has no controls. This results in much better behaviour in most scenarios:
Adding a small delay for hiding the hover actions on mouse leave, which makes it a lot easier to grab the drag handle:
Preventing the resize handle from overlapping Dashboard's stick top navigation:
Checklist
release_note:*label is applied per the guidelines