[EuiFlyoutResizable] Do not extend resize click area past flyout for flyouts with no overlays#8010
Merged
cee-chen merged 2 commits intoelastic:mainfrom Sep 9, 2024
Merged
Conversation
- flyouts with no overlay mask (push and ownFocus=false) - the goal of this is primarily to avoid avoid intercepting clicks on scrollbars or other content outside the flyout - keeping the centered behavior on flyouts w/ overlays, because those will close the entire flyout on outside click which is super disruptive, & we want to minimize that
Contributor
Author
|
@MichaelMarcialis Could I get a designer's UX review on this? I'm not sure whether the choice to keep the overlay flyout center-aligned was the right one (see PR description for rationale) or if consistency is more important. |
Collaborator
💚 Build Succeeded
|
mgadewoll
approved these changes
Sep 9, 2024
Contributor
mgadewoll
left a comment
There was a problem hiding this comment.
🚢 🐈⬛ Straight forward and clean implementation, LGTM! 🎉
Contributor
|
Thanks for the ping, @cee-chen. For That said, this issue has brought another question to mind: should we offer a prop to apply this kind of offset directly on the
|
Contributor
Author
|
@MichaelMarcialis Yes, I totally agree we should add a fix/prop for it for |
MichaelMarcialis
approved these changes
Sep 9, 2024
This was referenced Sep 12, 2024
Contributor
Author
|
Follow-up PR: #8021 |
Ikuni17
pushed a commit
to elastic/kibana
that referenced
this pull request
Sep 23, 2024
`v95.10.1`⏩`v95.11.0` > [!note] > The bulk of this release is **EuiDataGrid**. The component has been fully converted to Emotion, and several UX changes have been made to data cell actions. We recommend QA testing any data grid(s) that have custom styles applied to them. _[Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_ --- ## [`v95.11.0`](https://github.com/elastic/eui/releases/v95.11.0) - Updated `EuiText`, `EuiTextColor`, and `EuiTextAlign` with a new `component` prop that allows changing the default rendered `<div>` wrapper to a `<span>` or `<p>` tag. ([#7993](elastic/eui#7993)) - Updated `EuiDataGrid`'s cell actions to always consistently be left-aligned, regardless of text content alignment ([#8011](elastic/eui#8011)) - Increased `EuiDataGrid`'s cell actions hover zone to reduce UX friction when mousing over from the grid cell to its actions ([#8011](elastic/eui#8011)) **Bug fixes** - Fixed `EuiPopover` to correctly inherit from `EuiProvider`'s `componentDefaults.EuiPortal.insert` ([#8003](elastic/eui#8003)) - Fixed push `EuiFlyoutResizable`s to not potentially block scrollbars on outside content ([#8010](elastic/eui#8010)) - Fixed an `EuiDataGrid` bug where the `setCellProps` callback passed by `renderCellValue` was not correctly applying custom `data-test-subj`s ([#8011](elastic/eui#8011)) **Accessibility** - Updated the `EuiBasicTable` actions button's `aria-label` by adding a reference to the current row ([#7994](elastic/eui#7994)) **CSS-in-JS conversions** - Converted `EuiDataGrid`'s toolbar controls to Emotion ([#7997](elastic/eui#7997)) - Removed `$euiDataGridPopoverMaxHeight` - Converted `EuiDataGrid` to Emotion ([#7998](elastic/eui#7998)) - Removed `$euiZDataGrid` - Removed `$euiZHeaderBelowDataGrid` - Converted `EuiDataGrid`'s `gridStyle`s to Emotion; Removed the following Sass variables and mixins: ([#8006](elastic/eui#8006)) - `$euiDataGridCellPaddingS` - `$euiDataGridCellPaddingM` - `$euiDataGridCellPaddingL` - `$euiDataGridVerticalBorder` - `$euiDataGridPrefix` - `$euiDataGridStyles` - `@euiDataGridSelector` - `@euiDataGridStyles` - Converted `EuiDataGrid`'s cell popover, actions, and focus outline to Emotion; Removed the following Sass variables and mixins: ([#8011](elastic/eui#8011)) - `$euiZDataGridCellPopover` - `@euiDataGridCellFocus` - Converted `EuiDataGrid`'s row, header, and footer cells to Emotion; Removed the following Sass variables and mixins: ([#8013](elastic/eui#8013)) - `$euiDataGridColumnResizerWidth` - `@euiDataGridRowCell` - `@euiDataGridHeaderCell` - `@euiDataGridFooterCell`
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Sep 23, 2024
`v95.10.1`⏩`v95.11.0` > [!note] > The bulk of this release is **EuiDataGrid**. The component has been fully converted to Emotion, and several UX changes have been made to data cell actions. We recommend QA testing any data grid(s) that have custom styles applied to them. _[Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_ --- ## [`v95.11.0`](https://github.com/elastic/eui/releases/v95.11.0) - Updated `EuiText`, `EuiTextColor`, and `EuiTextAlign` with a new `component` prop that allows changing the default rendered `<div>` wrapper to a `<span>` or `<p>` tag. ([elastic#7993](elastic/eui#7993)) - Updated `EuiDataGrid`'s cell actions to always consistently be left-aligned, regardless of text content alignment ([elastic#8011](elastic/eui#8011)) - Increased `EuiDataGrid`'s cell actions hover zone to reduce UX friction when mousing over from the grid cell to its actions ([elastic#8011](elastic/eui#8011)) **Bug fixes** - Fixed `EuiPopover` to correctly inherit from `EuiProvider`'s `componentDefaults.EuiPortal.insert` ([elastic#8003](elastic/eui#8003)) - Fixed push `EuiFlyoutResizable`s to not potentially block scrollbars on outside content ([elastic#8010](elastic/eui#8010)) - Fixed an `EuiDataGrid` bug where the `setCellProps` callback passed by `renderCellValue` was not correctly applying custom `data-test-subj`s ([elastic#8011](elastic/eui#8011)) **Accessibility** - Updated the `EuiBasicTable` actions button's `aria-label` by adding a reference to the current row ([elastic#7994](elastic/eui#7994)) **CSS-in-JS conversions** - Converted `EuiDataGrid`'s toolbar controls to Emotion ([elastic#7997](elastic/eui#7997)) - Removed `$euiDataGridPopoverMaxHeight` - Converted `EuiDataGrid` to Emotion ([elastic#7998](elastic/eui#7998)) - Removed `$euiZDataGrid` - Removed `$euiZHeaderBelowDataGrid` - Converted `EuiDataGrid`'s `gridStyle`s to Emotion; Removed the following Sass variables and mixins: ([elastic#8006](elastic/eui#8006)) - `$euiDataGridCellPaddingS` - `$euiDataGridCellPaddingM` - `$euiDataGridCellPaddingL` - `$euiDataGridVerticalBorder` - `$euiDataGridPrefix` - `$euiDataGridStyles` - `@euiDataGridSelector` - `@euiDataGridStyles` - Converted `EuiDataGrid`'s cell popover, actions, and focus outline to Emotion; Removed the following Sass variables and mixins: ([elastic#8011](elastic/eui#8011)) - `$euiZDataGridCellPopover` - `@euiDataGridCellFocus` - Converted `EuiDataGrid`'s row, header, and footer cells to Emotion; Removed the following Sass variables and mixins: ([elastic#8013](elastic/eui#8013)) - `$euiDataGridColumnResizerWidth` - `@euiDataGridRowCell` - `@euiDataGridHeaderCell` - `@euiDataGridFooterCell` (cherry picked from commit 9bbb296)
kibanamachine
referenced
this pull request
in elastic/kibana
Sep 27, 2024
# Backport This will backport the following commits from `main` to `8.x`: - [Upgrade EUI to v95.11.0 (#192756)](#192756) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Cee Chen","email":"549407+cee-chen@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-09-23T22:45:19Z","message":"Upgrade EUI to v95.11.0 (#192756)\n\n`v95.10.1`⏩`v95.11.0`\r\n\r\n> [!note]\r\n> The bulk of this release is **EuiDataGrid**. The component has been\r\nfully converted to Emotion, and several UX changes have been made to\r\ndata cell actions. We recommend QA testing any data grid(s) that have\r\ncustom styles applied to them.\r\n\r\n_[Questions? Please see our Kibana upgrade\r\nFAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_\r\n\r\n---\r\n\r\n## [`v95.11.0`](https://github.com/elastic/eui/releases/v95.11.0)\r\n\r\n- Updated `EuiText`, `EuiTextColor`, and `EuiTextAlign` with a new\r\n`component` prop that allows changing the default rendered `<div>`\r\nwrapper to a `<span>` or `<p>` tag.\r\n([#7993](https://github.com/elastic/eui/pull/7993))\r\n- Updated `EuiDataGrid`'s cell actions to always consistently be\r\nleft-aligned, regardless of text content alignment\r\n([#8011](https://github.com/elastic/eui/pull/8011))\r\n- Increased `EuiDataGrid`'s cell actions hover zone to reduce UX\r\nfriction when mousing over from the grid cell to its actions\r\n([#8011](https://github.com/elastic/eui/pull/8011))\r\n\r\n**Bug fixes**\r\n\r\n- Fixed `EuiPopover` to correctly inherit from `EuiProvider`'s\r\n`componentDefaults.EuiPortal.insert`\r\n([#8003](https://github.com/elastic/eui/pull/8003))\r\n- Fixed push `EuiFlyoutResizable`s to not potentially block scrollbars\r\non outside content ([#8010](https://github.com/elastic/eui/pull/8010))\r\n- Fixed an `EuiDataGrid` bug where the `setCellProps` callback passed by\r\n`renderCellValue` was not correctly applying custom `data-test-subj`s\r\n([#8011](https://github.com/elastic/eui/pull/8011))\r\n\r\n**Accessibility**\r\n\r\n- Updated the `EuiBasicTable` actions button's `aria-label` by adding a\r\nreference to the current row\r\n([#7994](https://github.com/elastic/eui/pull/7994))\r\n\r\n**CSS-in-JS conversions**\r\n\r\n- Converted `EuiDataGrid`'s toolbar controls to Emotion\r\n([#7997](https://github.com/elastic/eui/pull/7997))\r\n - Removed `$euiDataGridPopoverMaxHeight`\r\n- Converted `EuiDataGrid` to Emotion\r\n([#7998](https://github.com/elastic/eui/pull/7998))\r\n - Removed `$euiZDataGrid`\r\n - Removed `$euiZHeaderBelowDataGrid`\r\n- Converted `EuiDataGrid`'s `gridStyle`s to Emotion; Removed the\r\nfollowing Sass variables and mixins:\r\n([#8006](https://github.com/elastic/eui/pull/8006))\r\n - `$euiDataGridCellPaddingS`\r\n - `$euiDataGridCellPaddingM`\r\n - `$euiDataGridCellPaddingL`\r\n - `$euiDataGridVerticalBorder`\r\n - `$euiDataGridPrefix`\r\n - `$euiDataGridStyles`\r\n - `@euiDataGridSelector`\r\n - `@euiDataGridStyles`\r\n- Converted `EuiDataGrid`'s cell popover, actions, and focus outline to\r\nEmotion; Removed the following Sass variables and mixins:\r\n([#8011](https://github.com/elastic/eui/pull/8011))\r\n - `$euiZDataGridCellPopover`\r\n - `@euiDataGridCellFocus`\r\n- Converted `EuiDataGrid`'s row, header, and footer cells to Emotion;\r\nRemoved the following Sass variables and mixins:\r\n([#8013](https://github.com/elastic/eui/pull/8013))\r\n - `$euiDataGridColumnResizerWidth`\r\n - `@euiDataGridRowCell`\r\n - `@euiDataGridHeaderCell`\r\n - `@euiDataGridFooterCell`","sha":"9bbb296078ea385561d46819001644cdb4fdc714","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","EUI","v9.0.0","ci:project-deploy-observability","Team:obs-ux-management","v8.16.0","backport:version"],"title":"Upgrade EUI to v95.11.0","number":192756,"url":"https://github.com/elastic/kibana/pull/192756","mergeCommit":{"message":"Upgrade EUI to v95.11.0 (#192756)\n\n`v95.10.1`⏩`v95.11.0`\r\n\r\n> [!note]\r\n> The bulk of this release is **EuiDataGrid**. The component has been\r\nfully converted to Emotion, and several UX changes have been made to\r\ndata cell actions. We recommend QA testing any data grid(s) that have\r\ncustom styles applied to them.\r\n\r\n_[Questions? Please see our Kibana upgrade\r\nFAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_\r\n\r\n---\r\n\r\n## [`v95.11.0`](https://github.com/elastic/eui/releases/v95.11.0)\r\n\r\n- Updated `EuiText`, `EuiTextColor`, and `EuiTextAlign` with a new\r\n`component` prop that allows changing the default rendered `<div>`\r\nwrapper to a `<span>` or `<p>` tag.\r\n([#7993](https://github.com/elastic/eui/pull/7993))\r\n- Updated `EuiDataGrid`'s cell actions to always consistently be\r\nleft-aligned, regardless of text content alignment\r\n([#8011](https://github.com/elastic/eui/pull/8011))\r\n- Increased `EuiDataGrid`'s cell actions hover zone to reduce UX\r\nfriction when mousing over from the grid cell to its actions\r\n([#8011](https://github.com/elastic/eui/pull/8011))\r\n\r\n**Bug fixes**\r\n\r\n- Fixed `EuiPopover` to correctly inherit from `EuiProvider`'s\r\n`componentDefaults.EuiPortal.insert`\r\n([#8003](https://github.com/elastic/eui/pull/8003))\r\n- Fixed push `EuiFlyoutResizable`s to not potentially block scrollbars\r\non outside content ([#8010](https://github.com/elastic/eui/pull/8010))\r\n- Fixed an `EuiDataGrid` bug where the `setCellProps` callback passed by\r\n`renderCellValue` was not correctly applying custom `data-test-subj`s\r\n([#8011](https://github.com/elastic/eui/pull/8011))\r\n\r\n**Accessibility**\r\n\r\n- Updated the `EuiBasicTable` actions button's `aria-label` by adding a\r\nreference to the current row\r\n([#7994](https://github.com/elastic/eui/pull/7994))\r\n\r\n**CSS-in-JS conversions**\r\n\r\n- Converted `EuiDataGrid`'s toolbar controls to Emotion\r\n([#7997](https://github.com/elastic/eui/pull/7997))\r\n - Removed `$euiDataGridPopoverMaxHeight`\r\n- Converted `EuiDataGrid` to Emotion\r\n([#7998](https://github.com/elastic/eui/pull/7998))\r\n - Removed `$euiZDataGrid`\r\n - Removed `$euiZHeaderBelowDataGrid`\r\n- Converted `EuiDataGrid`'s `gridStyle`s to Emotion; Removed the\r\nfollowing Sass variables and mixins:\r\n([#8006](https://github.com/elastic/eui/pull/8006))\r\n - `$euiDataGridCellPaddingS`\r\n - `$euiDataGridCellPaddingM`\r\n - `$euiDataGridCellPaddingL`\r\n - `$euiDataGridVerticalBorder`\r\n - `$euiDataGridPrefix`\r\n - `$euiDataGridStyles`\r\n - `@euiDataGridSelector`\r\n - `@euiDataGridStyles`\r\n- Converted `EuiDataGrid`'s cell popover, actions, and focus outline to\r\nEmotion; Removed the following Sass variables and mixins:\r\n([#8011](https://github.com/elastic/eui/pull/8011))\r\n - `$euiZDataGridCellPopover`\r\n - `@euiDataGridCellFocus`\r\n- Converted `EuiDataGrid`'s row, header, and footer cells to Emotion;\r\nRemoved the following Sass variables and mixins:\r\n([#8013](https://github.com/elastic/eui/pull/8013))\r\n - `$euiDataGridColumnResizerWidth`\r\n - `@euiDataGridRowCell`\r\n - `@euiDataGridHeaderCell`\r\n - `@euiDataGridFooterCell`","sha":"9bbb296078ea385561d46819001644cdb4fdc714"}},"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/192756","number":192756,"mergeCommit":{"message":"Upgrade EUI to v95.11.0 (#192756)\n\n`v95.10.1`⏩`v95.11.0`\r\n\r\n> [!note]\r\n> The bulk of this release is **EuiDataGrid**. The component has been\r\nfully converted to Emotion, and several UX changes have been made to\r\ndata cell actions. We recommend QA testing any data grid(s) that have\r\ncustom styles applied to them.\r\n\r\n_[Questions? Please see our Kibana upgrade\r\nFAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_\r\n\r\n---\r\n\r\n## [`v95.11.0`](https://github.com/elastic/eui/releases/v95.11.0)\r\n\r\n- Updated `EuiText`, `EuiTextColor`, and `EuiTextAlign` with a new\r\n`component` prop that allows changing the default rendered `<div>`\r\nwrapper to a `<span>` or `<p>` tag.\r\n([#7993](https://github.com/elastic/eui/pull/7993))\r\n- Updated `EuiDataGrid`'s cell actions to always consistently be\r\nleft-aligned, regardless of text content alignment\r\n([#8011](https://github.com/elastic/eui/pull/8011))\r\n- Increased `EuiDataGrid`'s cell actions hover zone to reduce UX\r\nfriction when mousing over from the grid cell to its actions\r\n([#8011](https://github.com/elastic/eui/pull/8011))\r\n\r\n**Bug fixes**\r\n\r\n- Fixed `EuiPopover` to correctly inherit from `EuiProvider`'s\r\n`componentDefaults.EuiPortal.insert`\r\n([#8003](https://github.com/elastic/eui/pull/8003))\r\n- Fixed push `EuiFlyoutResizable`s to not potentially block scrollbars\r\non outside content ([#8010](https://github.com/elastic/eui/pull/8010))\r\n- Fixed an `EuiDataGrid` bug where the `setCellProps` callback passed by\r\n`renderCellValue` was not correctly applying custom `data-test-subj`s\r\n([#8011](https://github.com/elastic/eui/pull/8011))\r\n\r\n**Accessibility**\r\n\r\n- Updated the `EuiBasicTable` actions button's `aria-label` by adding a\r\nreference to the current row\r\n([#7994](https://github.com/elastic/eui/pull/7994))\r\n\r\n**CSS-in-JS conversions**\r\n\r\n- Converted `EuiDataGrid`'s toolbar controls to Emotion\r\n([#7997](https://github.com/elastic/eui/pull/7997))\r\n - Removed `$euiDataGridPopoverMaxHeight`\r\n- Converted `EuiDataGrid` to Emotion\r\n([#7998](https://github.com/elastic/eui/pull/7998))\r\n - Removed `$euiZDataGrid`\r\n - Removed `$euiZHeaderBelowDataGrid`\r\n- Converted `EuiDataGrid`'s `gridStyle`s to Emotion; Removed the\r\nfollowing Sass variables and mixins:\r\n([#8006](https://github.com/elastic/eui/pull/8006))\r\n - `$euiDataGridCellPaddingS`\r\n - `$euiDataGridCellPaddingM`\r\n - `$euiDataGridCellPaddingL`\r\n - `$euiDataGridVerticalBorder`\r\n - `$euiDataGridPrefix`\r\n - `$euiDataGridStyles`\r\n - `@euiDataGridSelector`\r\n - `@euiDataGridStyles`\r\n- Converted `EuiDataGrid`'s cell popover, actions, and focus outline to\r\nEmotion; Removed the following Sass variables and mixins:\r\n([#8011](https://github.com/elastic/eui/pull/8011))\r\n - `$euiZDataGridCellPopover`\r\n - `@euiDataGridCellFocus`\r\n- Converted `EuiDataGrid`'s row, header, and footer cells to Emotion;\r\nRemoved the following Sass variables and mixins:\r\n([#8013](https://github.com/elastic/eui/pull/8013))\r\n - `$euiDataGridColumnResizerWidth`\r\n - `@euiDataGridRowCell`\r\n - `@euiDataGridHeaderCell`\r\n - `@euiDataGridFooterCell`","sha":"9bbb296078ea385561d46819001644cdb4fdc714"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> --------- Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com> Co-authored-by: Cee Chen <constance.chen@elastic.co>
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
4 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.
Summary
closes #7988
This PR fixes the scenario where the content to the left of the resizable flyout has a scrollbar (or any other clickable element that butts up against the flyout), and the placement of the resize border/indicator button prevents the thing it's overlaying from being clicked:
FYI that I kept the centered styling on flyouts with overlay masks, because those overlays will close the entire flyout on outside click which feels super disruptive - I felt the extra space there would be more beneficial UX-wise to help minimize that for users who attempt to resize and are a few pixels off.
QA
ownFocuscontrol tofalsetypetopushGeneral checklist
- [ ] Checked in mobile- [ ] Added or updated jest and cypress tests- [ ] If applicable, added the breaking change issue label (and filled out the breaking change checklist)