Skip to content

Commit

Permalink
[8.x] [Dashboard Usability] Remove borders from presentation panel an…
Browse files Browse the repository at this point in the history
…d CSS clean up for hover actions (#198454) (#200051)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Dashboard Usability] Remove borders from presentation panel and CSS
clean up for hover actions
(#198454)](#198454)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Catherine
Liu","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-13T16:46:30Z","message":"[Dashboard
Usability] Remove borders from presentation panel and CSS clean up for
hover actions (#198454)\n\n## Summary\r\n\r\nCloses
#198370. \r\nFollow up to
#182535.\r\n\r\nThis removes border style overrides from the apps that
support\r\nembeddables and cleans up outline styles for presentation
panel\r\nborders/hover actions. I removed all of the dashboard specific
border\r\nstyles and made them more generic for any embeddable panel.
Now, the\r\nborders respect the `showBorder` prop on the embeddable
input. However\r\neven when `showBorder` is `false`, a border is shown
on focus/hover to\r\nalign with the border styles of the hover
actions.\r\n\r\n### Checklist\r\n\r\nDelete any items that are not
applicable to this PR.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [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- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n-
[ ] This will appear in the **Release Notes** and follow
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
<[email protected]>","sha":"9af4bb5b7f70c9f281f728b24292e89171dcc846","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Embedding","release_note:fix","Team:Presentation","loe:medium","impact:high","v9.0.0","ci:project-deploy-observability","ci:project-deploy-security","Team:obs-ux-management","backport:version","v8.17.0"],"title":"[Dashboard
Usability] Remove borders from presentation panel and CSS clean up for
hover
actions","number":198454,"url":"https://github.com/elastic/kibana/pull/198454","mergeCommit":{"message":"[Dashboard
Usability] Remove borders from presentation panel and CSS clean up for
hover actions (#198454)\n\n## Summary\r\n\r\nCloses
#198370. \r\nFollow up to
#182535.\r\n\r\nThis removes border style overrides from the apps that
support\r\nembeddables and cleans up outline styles for presentation
panel\r\nborders/hover actions. I removed all of the dashboard specific
border\r\nstyles and made them more generic for any embeddable panel.
Now, the\r\nborders respect the `showBorder` prop on the embeddable
input. However\r\neven when `showBorder` is `false`, a border is shown
on focus/hover to\r\nalign with the border styles of the hover
actions.\r\n\r\n### Checklist\r\n\r\nDelete any items that are not
applicable to this PR.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [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- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n-
[ ] This will appear in the **Release Notes** and follow
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
<[email protected]>","sha":"9af4bb5b7f70c9f281f728b24292e89171dcc846"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/198454","number":198454,"mergeCommit":{"message":"[Dashboard
Usability] Remove borders from presentation panel and CSS clean up for
hover actions (#198454)\n\n## Summary\r\n\r\nCloses
#198370. \r\nFollow up to
#182535.\r\n\r\nThis removes border style overrides from the apps that
support\r\nembeddables and cleans up outline styles for presentation
panel\r\nborders/hover actions. I removed all of the dashboard specific
border\r\nstyles and made them more generic for any embeddable panel.
Now, the\r\nborders respect the `showBorder` prop on the embeddable
input. However\r\neven when `showBorder` is `false`, a border is shown
on focus/hover to\r\nalign with the border styles of the hover
actions.\r\n\r\n### Checklist\r\n\r\nDelete any items that are not
applicable to this PR.\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [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- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\r\n-
[ ] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n\r\n###
Risk Matrix\r\n\r\nDelete this section if it is not applicable to this
PR.\r\n\r\nBefore closing this PR, invite QA, stakeholders, and other
developers to\r\nidentify risks that should be tested prior to the
change/feature\r\nrelease.\r\n\r\nWhen forming the risk matrix, consider
some of the following examples\r\nand how they may potentially impact
the change:\r\n\r\n| Risk | Probability | Severity | Mitigation/Notes
|\r\n\r\n|---------------------------|-------------|----------|-------------------------|\r\n|
Multiple Spaces&mdash;unexpected behavior in non-default Kibana
Space.\r\n| Low | High | Integration tests will verify that all features
are still\r\nsupported in non-default Kibana Space and when user
switches between\r\nspaces. |\r\n| Multiple nodes&mdash;Elasticsearch
polling might have race conditions\r\nwhen multiple Kibana nodes are
polling for the same tasks. | High | Low\r\n| Tasks are idempotent, so
executing them multiple times will not result\r\nin logical error, but
will degrade performance. To test for this case we\r\nadd plenty of unit
tests around this logic and document manual testing\r\nprocedure. |\r\n|
Code should gracefully handle cases when feature X or plugin Y
are\r\ndisabled. | Medium | High | Unit tests will verify that any
feature flag\r\nor plugin combination still results in our service
operational. |\r\n| [See more potential
risk\r\nexamples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx)
|\r\n\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for
breaking API changes and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n-
[ ] This will appear in the **Release Notes** and follow
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
<[email protected]>","sha":"9af4bb5b7f70c9f281f728b24292e89171dcc846"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Catherine Liu <[email protected]>
  • Loading branch information
kibanamachine and cqliu1 authored Nov 13, 2024
1 parent 1b38ec1 commit 9b162c5
Show file tree
Hide file tree
Showing 12 changed files with 165 additions and 243 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,12 @@
z-index: $euiZLevel9;
top: -$euiSizeXL;

// Show hover actions with drag handle
.embPanel__hoverActions:has(.embPanel--dragHandle) {
opacity: 1;
}

// Hide hover actions without drag handle
.embPanel__hoverActions:not(:has(.embPanel--dragHandle)) {
opacity: 0;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,44 +1,11 @@
/**
* EDITING MODE
* Use .dshLayout--editing to target editing state because
* .embPanel--editing doesn't get updating without a hard refresh
*/

.dshLayout--editing {
// change the style of the hover actions border to a dashed line in edit mode
.embPanel__hoverActionsAnchor {
.embPanel__hoverActionsWrapper {
.embPanel__hoverActions {
border-color: $euiColorMediumShade;
border-style: dashed;
}
}
}
}

// LAYOUT MODES
// Adjust borders/etc... for non-spaced out and expanded panels
.dshLayout-withoutMargins {
.embPanel,
.embPanel__hoverActionsAnchor {
box-shadow: none;
outline: none;
border-radius: 0;
}

&.dshLayout--editing {
.embPanel__hoverActionsAnchor:hover {
outline: 1px dashed $euiColorMediumShade;
}
}

.embPanel__hoverActionsAnchor:hover {
outline: $euiBorderThin;
z-index: $euiZLevel2;
}
margin-top: $euiSizeS;

.embPanel__content,
.dshDashboardGrid__item--highlighted,
.embPanel,
.embPanel__hoverActionsAnchor,
.lnsExpressionRenderer {
border-radius: 0;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ Default.args = {
hideHeader: false,
loading: false,
showShadow: false,
showBorder: true,
showBorder: false,
title: 'Hello World',
viewMode: true,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export const getLensAttributes = (group: EventAnnotationGroupConfig, timeField:
language: 'kuery',
},
filters: [],
showBorder: false,
datasourceStates: {
formBased: {
layers: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
height: 100%;
min-height: $euiSizeL + 2px; // + 2px to account for border
position: relative;
border: none;
outline: $euiBorderThin;

&-isLoading {
// completely center the loading indicator
Expand Down Expand Up @@ -105,47 +103,9 @@
}
}

// OPTIONS MENU

/**
* 1. Use opacity to make this element accessible to screen readers and keyboard.
* 2. Show on focus to enable keyboard accessibility.
* 3. Always show in editing mode
*/

.embPanel__optionsMenuButton {
background-color: transparentize($euiColorLightShade, .5);
border-bottom-right-radius: 0;
border-top-left-radius: 0;

&:focus {
background-color: transparentize($euiColorLightestShade, .5);
}
}

.embPanel__optionsMenuPopover-loading {
width: $euiSizeS * 32;
}

.embPanel__optionsMenuPopover-notification::after {
position: absolute;
top: 0;
right: 0;
content: '';
transform: translate(50%, -50%);
color: $euiColorAccent;
font-size: $euiSizeL;
}

// EDITING MODE

.embPanel--editing {
transition: all $euiAnimSpeedFast $euiAnimSlightResistance;
outline: 1px dashed $euiColorMediumShade;

.embPanel--dragHandle {
transition: background-color $euiAnimSpeedFast $euiAnimSlightResistance;

.embPanel--dragHandle:hover {
background-color: transparentize($euiColorWarning, lightOrDarkTheme(.9, .7));
cursor: move;
Expand All @@ -170,56 +130,14 @@
z-index: $euiZLevel1;
}

.embPanel__hoverActionsAnchor {
position: relative;
height: 100%;

.embPanel__hoverActionsWrapper {
height: $euiSizeXL;
position: absolute;
top: 0;
display: flex;
justify-content: space-between;
padding: 0 $euiSize;
flex-wrap: nowrap;
min-width: 100%;
z-index: -1;
pointer-events: none; // Prevent hover actions wrapper from blocking interactions with other panels
}

.embPanel__hoverActions {
opacity: 0;
padding: calc($euiSizeXS - 1px);
display: flex;
flex-wrap: nowrap;
border: $euiBorderThin;

background-color: $euiColorEmptyShade;
height: $euiSizeXL;

pointer-events: all; // Re-enable pointer-events for hover actions
}

.embPanel--dragHandle {
cursor: move;
.embPanel--dragHandle {
cursor: move;

img {
pointer-events: all !important;
}
img {
pointer-events: all !important;
}
}

.embPanel__descriptionTooltipAnchor {
padding: $euiSizeXS;
}

&:hover .embPanel__hoverActionsWrapper,
&:focus-within .embPanel__hoverActionsWrapper,
.embPanel__hoverActionsWrapper--lockHoverActions {
z-index: $euiZLevel9;
top: -$euiSizeXL;

.embPanel__hoverActions {
opacity: 1;
}
}
}
.embPanel__descriptionTooltipAnchor {
padding: $euiSizeXS;
}
Loading

0 comments on commit 9b162c5

Please sign in to comment.