-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[EUI] Add scrollLock
workaround CSS to Kibana's body
#153227
Conversation
@Heenawter Do you want to keep d59faae in this PR, or leave it out for now and wait for elastic/eui#6645 to merge/land in Kibana? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh boy, this was a fun one 🙃 Change discussed at length - LGTM 👍
Sounds good, thanks! We'll handle reverting d59faae in our next EUI upgrade! |
Hm, I'm not sure your draft approval counted for some mysterious GitHub reason - you may have to reapprove 😖 |
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🎉
I'm approving based on the explanation which makes sense to me.
Thanks y'all! |
💚 Build Succeeded
Metrics [docs]Page load bundle
Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
…loy-my-kibana-oblt * upstream/main: (727 commits) Upgrade caniuse-lite db (elastic#153318) [Security Solution] expanded flyout - right section - json tab implementation (elastic#152935) chore(slo): Make APM indicator's index required (elastic#153311) skip failing test suite (elastic#136688) [Security Solution] Fix security-solution storybook package codeowners (elastic#153307) [EUI] Add `scrollLock` workaround CSS to Kibana's `body` (elastic#153227) [Cloud Security] Show coming soon deployments of vulnerability management (elastic#153249) [Cloud Security] fixed onboarding link directs to cspm integration (elastic#153268) [Response Ops][Alerting] Reusable functions for FAAD resource installation (elastic#152849) remove geohash_grid aggregation support (elastic#152952) [Tech Debt] Reorder Rules page (elastic#152897) [Saved Object Finder] Add help text & left button (elastic#152742) [Transform] Replace SavedObjectsFinder component (elastic#153128) Make pipeline creation endpoint accept a full pipeline definition (elastic#153133) [Fleet] Displaying policy changes in Agent activity (elastic#153237) skip flaky suite (elastic#152852) [Security Solution][Endpoint] Add tests to cover RBAC entries in the Role Kibana Privileges flyout (elastic#153068) [Security Solution][Endpoint] Additional tests for Response Console History Log page (covers TestRail manual tests) (elastic#153042) [Monitoring] Display node roles in Nodes table (elastic#152127) Rename getEditAlertFlyout to getEditRuleFlyout (elastic#153243) ...
## Summary `[email protected]` ⏩ `[email protected]` This upgrade consists of a backport release intended to fix a major bug where portals within `EuiFlyout`s and `EuiModal`s are not scrollable. fixes #156161 This release also adds functionality that resolves the need for a TODO workaround added in #153227 --- ## [`77.1.2`](https://github.com/elastic/eui/tree/v77.1.2) - Updated `EuiFocusTrap` to support the `gapMode` prop configuration (now defaults to `padding`) ([#6744](elastic/eui#6744)) **Bug fixes** - Fixed the `scrollLock` property on `EuiFocusTrap` (and other components using `EuiFocusTrap`, such as `EuiFlyout` and `EuiModal`) to no longer block scrolling on nested portalled content, such as combobox dropdowns ([#6744](elastic/eui#6744)) --------- Co-authored-by: Kibana Machine <[email protected]>
## Summary `[email protected]` ⏩ `[email protected]` This upgrade consists of a backport release intended to fix a major bug where portals within `EuiFlyout`s and `EuiModal`s are not scrollable. fixes elastic#156161 This release also adds functionality that resolves the need for a TODO workaround added in elastic#153227 --- ## [`77.1.2`](https://github.com/elastic/eui/tree/v77.1.2) - Updated `EuiFocusTrap` to support the `gapMode` prop configuration (now defaults to `padding`) ([elastic#6744](elastic/eui#6744)) **Bug fixes** - Fixed the `scrollLock` property on `EuiFocusTrap` (and other components using `EuiFocusTrap`, such as `EuiFlyout` and `EuiModal`) to no longer block scrolling on nested portalled content, such as combobox dropdowns ([elastic#6744](elastic/eui#6744)) --------- Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit 0564e54)
# Backport This will backport the following commits from `main` to `8.8`: - [Upgrade EUI to v77.1.2 (#156232)](#156232) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Cee Chen","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-05-01T17:05:11Z","message":"Upgrade EUI to v77.1.2 (#156232)\n\n## Summary\r\n\r\n`[email protected]` ⏩ `[email protected]`\r\n\r\nThis upgrade consists of a backport release intended to fix a major bug\r\nwhere portals within `EuiFlyout`s and `EuiModal`s are not scrollable.\r\nfixes https://github.com/elastic/kibana/issues/156161\r\n\r\nThis release also adds functionality that resolves the need for a TODO\r\nworkaround added in https://github.com/elastic/kibana/pull/153227\r\n\r\n---\r\n\r\n## [`77.1.2`](https://github.com/elastic/eui/tree/v77.1.2)\r\n\r\n- Updated `EuiFocusTrap` to support the `gapMode` prop configuration\r\n(now defaults to `padding`)\r\n([#6744](https://github.com/elastic/eui/pull/6744))\r\n\r\n**Bug fixes**\r\n\r\n- Fixed the `scrollLock` property on `EuiFocusTrap` (and other\r\ncomponents using `EuiFocusTrap`, such as `EuiFlyout` and `EuiModal`) to\r\nno longer block scrolling on nested portalled content, such as combobox\r\ndropdowns ([#6744](https://github.com/elastic/eui/pull/6744))\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"0564e5434e56ca24c7cbb3fbf4a58d6242714e71","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","EUI","v8.8.0","v8.9.0"],"number":156232,"url":"https://github.com/elastic/kibana/pull/156232","mergeCommit":{"message":"Upgrade EUI to v77.1.2 (#156232)\n\n## Summary\r\n\r\n`[email protected]` ⏩ `[email protected]`\r\n\r\nThis upgrade consists of a backport release intended to fix a major bug\r\nwhere portals within `EuiFlyout`s and `EuiModal`s are not scrollable.\r\nfixes https://github.com/elastic/kibana/issues/156161\r\n\r\nThis release also adds functionality that resolves the need for a TODO\r\nworkaround added in https://github.com/elastic/kibana/pull/153227\r\n\r\n---\r\n\r\n## [`77.1.2`](https://github.com/elastic/eui/tree/v77.1.2)\r\n\r\n- Updated `EuiFocusTrap` to support the `gapMode` prop configuration\r\n(now defaults to `padding`)\r\n([#6744](https://github.com/elastic/eui/pull/6744))\r\n\r\n**Bug fixes**\r\n\r\n- Fixed the `scrollLock` property on `EuiFocusTrap` (and other\r\ncomponents using `EuiFocusTrap`, such as `EuiFlyout` and `EuiModal`) to\r\nno longer block scrolling on nested portalled content, such as combobox\r\ndropdowns ([#6744](https://github.com/elastic/eui/pull/6744))\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"0564e5434e56ca24c7cbb3fbf4a58d6242714e71"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/156232","number":156232,"mergeCommit":{"message":"Upgrade EUI to v77.1.2 (#156232)\n\n## Summary\r\n\r\n`[email protected]` ⏩ `[email protected]`\r\n\r\nThis upgrade consists of a backport release intended to fix a major bug\r\nwhere portals within `EuiFlyout`s and `EuiModal`s are not scrollable.\r\nfixes https://github.com/elastic/kibana/issues/156161\r\n\r\nThis release also adds functionality that resolves the need for a TODO\r\nworkaround added in https://github.com/elastic/kibana/pull/153227\r\n\r\n---\r\n\r\n## [`77.1.2`](https://github.com/elastic/eui/tree/v77.1.2)\r\n\r\n- Updated `EuiFocusTrap` to support the `gapMode` prop configuration\r\n(now defaults to `padding`)\r\n([#6744](https://github.com/elastic/eui/pull/6744))\r\n\r\n**Bug fixes**\r\n\r\n- Fixed the `scrollLock` property on `EuiFocusTrap` (and other\r\ncomponents using `EuiFocusTrap`, such as `EuiFlyout` and `EuiModal`) to\r\nno longer block scrolling on nested portalled content, such as combobox\r\ndropdowns ([#6744](https://github.com/elastic/eui/pull/6744))\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"0564e5434e56ca24c7cbb3fbf4a58d6242714e71"}}]}] BACKPORT--> Co-authored-by: Cee Chen <[email protected]>
Summary
The
scrollLock
property onEuiFocusTrap
"freezes" the width of the of the body so that the removal/addition of the scrollbar when a full screen focus trap opens (e.g. a modal, flyout, or other fullscreen mode) doesn't cause the page width to jump/rerender.It turns however, that this behavior (which sets a
margin-right
on thebody
for the width of the scrollbar) does not work as expected in Kibana due to its existingwidth: 100%; min-width: 100%
CSS on itsbody
. As such, we need to temporarily work around this by settingpadding-right
instead until theKashey/react-focus-on#65 is fixed.d59faae can be reverted if not desired in this PR.
Checklist