Skip to content
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

[EuiFlyout] Preserve scrollbar width on body when EuiFlyout is open #6645

Merged
merged 3 commits into from
Mar 20, 2023

Conversation

cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Mar 15, 2023

Summary

The scrollLock property on EuiFocusTrap (actually an underlying 3rd party library, react-focus-on) allows preventing scrolling on the background and additionally "freezes" the width of the of the body so that the removal/addition of the scrollbar doesn't cause the page width to change slightly whenever the focus trap opens:

This property was added for EuiModal in daf6a73. This PR adds this functionality & behavior as well to all other components with focus traps with full-screen behavior, i.e. EuiFlyout, EuiImage, and EuiCodeBlock.

Note that EuiDataGrid's fullscreen mode is not handled here, as page jumping is going to happen no matter what in EuiDataGrid (the entire grid is taken out of its normal page flow and becomes fullscreen/fixed), so it's not super relevant or useful to add scrollLock to it.

QA

General checklist

  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • A changelog entry exists and is marked appropriately
    - [ ] Added or updated jest and cypress tests N/A - not added since this is a third party library

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6645/

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_6645/

@cee-chen cee-chen changed the title [EuiFlyout] Preserve scrollbar width on body when EuiFlyout [EuiFlyout] Preserve scrollbar width on body when EuiFlyout is open Mar 15, 2023
@cee-chen cee-chen requested a review from breehall March 20, 2023 15:58
@cee-chen
Copy link
Contributor Author

Hey @breehall! Could you review/approve this small PR today? Thanks!

@cee-chen
Copy link
Contributor Author

Oops, just saw you're OOO, apologies. Going to tag someone else

@cee-chen cee-chen removed the request for review from breehall March 20, 2023 16:16
@elizabetdev elizabetdev self-requested a review March 20, 2023 16:21
Copy link
Contributor

@elizabetdev elizabetdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @cee-chen.

Tested in Chrome, Safari and Firefox, and LGTM! 🎉

@cee-chen cee-chen merged commit ec75a84 into elastic:main Mar 20, 2023
@cee-chen cee-chen deleted the flyout/scroll-lock branch March 20, 2023 16:52
1Copenut added a commit to elastic/kibana that referenced this pull request Mar 23, 2023
## Summary

eui@76.0.2 ⏩ eui@76.3.0

## [`76.3.0`](https://github.com/elastic/eui/tree/v76.3.0)

- Updated `EuiSkipLink`'s `fallbackDestination` prop to support an array
of query selector strings
([#6646](elastic/eui#6646))

**Bug fixes**

- Fixed `EuiFlyout` to preserve body scrollbar width on open
([#6645](elastic/eui#6645))
- Fixed `EuiImage`'s full screen mode to not scroll jump & to preserve
body scrollbar width on open
([#6645](elastic/eui#6645))
- Fixed `EuiCodeBlock`'s full screen mode to not scroll jump & to
preserve body scrollbar width on open
([#6645](elastic/eui#6645))

## [`76.2.0`](https://github.com/elastic/eui/tree/v76.2.0)

- Added new `renderCustomGridBody` escape hatch rendering prop to
`EuiDataGrid` ([#6624](elastic/eui#6624))

**Bug fixes**

- Fixed visual listbox focus ring bug on non-searchable `EuiSelectable`s
([#6637](elastic/eui#6637))
- Added a legacy `alert` alias for the `warning` `EuiIcon` type
([#6640](elastic/eui#6640))
- Fixed a type definition incorrectly coming from a dev dependency,
which was causing issues for some consuming projects
([#6643](elastic/eui#6643))

## [`76.1.0`](https://github.com/elastic/eui/tree/v76.1.0)

- Added more detailed screen reader instructions to `EuiSelectable`,
`EuiSuggest`, `EuiSelectableTemplateSitewide`, `EuiRange`, and
`EuiDualRange`. ([#6589](elastic/eui#6589))
- Added new `placeholder` prop to `EuiSuperSelect`
([#6630](elastic/eui#6630))
- Added new `setCellPopoverProps` parameter callback to `EuiDataGrid`'s
`renderCellPopover` prop
([#6632](elastic/eui#6632))

**Bug fixes**

- Fixed an ARIA attribute in `EuiSelectableList`
([#6589](elastic/eui#6589))
- Fixed `EuiSelectable` to no longer show active selection state or
respond to the Up/Down arrow keys when focus is inside the selectable
container, but not on the searchbox or listbox.
([#6631](elastic/eui#6631))

---------

Co-authored-by: Tiago Costa <tiago.costa@elastic.co>
jennypavlova pushed a commit to jennypavlova/kibana that referenced this pull request Mar 24, 2023
## Summary

eui@76.0.2 ⏩ eui@76.3.0

## [`76.3.0`](https://github.com/elastic/eui/tree/v76.3.0)

- Updated `EuiSkipLink`'s `fallbackDestination` prop to support an array
of query selector strings
([elastic#6646](elastic/eui#6646))

**Bug fixes**

- Fixed `EuiFlyout` to preserve body scrollbar width on open
([elastic#6645](elastic/eui#6645))
- Fixed `EuiImage`'s full screen mode to not scroll jump & to preserve
body scrollbar width on open
([elastic#6645](elastic/eui#6645))
- Fixed `EuiCodeBlock`'s full screen mode to not scroll jump & to
preserve body scrollbar width on open
([elastic#6645](elastic/eui#6645))

## [`76.2.0`](https://github.com/elastic/eui/tree/v76.2.0)

- Added new `renderCustomGridBody` escape hatch rendering prop to
`EuiDataGrid` ([elastic#6624](elastic/eui#6624))

**Bug fixes**

- Fixed visual listbox focus ring bug on non-searchable `EuiSelectable`s
([elastic#6637](elastic/eui#6637))
- Added a legacy `alert` alias for the `warning` `EuiIcon` type
([elastic#6640](elastic/eui#6640))
- Fixed a type definition incorrectly coming from a dev dependency,
which was causing issues for some consuming projects
([elastic#6643](elastic/eui#6643))

## [`76.1.0`](https://github.com/elastic/eui/tree/v76.1.0)

- Added more detailed screen reader instructions to `EuiSelectable`,
`EuiSuggest`, `EuiSelectableTemplateSitewide`, `EuiRange`, and
`EuiDualRange`. ([elastic#6589](elastic/eui#6589))
- Added new `placeholder` prop to `EuiSuperSelect`
([elastic#6630](elastic/eui#6630))
- Added new `setCellPopoverProps` parameter callback to `EuiDataGrid`'s
`renderCellPopover` prop
([elastic#6632](elastic/eui#6632))

**Bug fixes**

- Fixed an ARIA attribute in `EuiSelectableList`
([elastic#6589](elastic/eui#6589))
- Fixed `EuiSelectable` to no longer show active selection state or
respond to the Up/Down arrow keys when focus is inside the selectable
container, but not on the searchbox or listbox.
([elastic#6631](elastic/eui#6631))

---------

Co-authored-by: Tiago Costa <tiago.costa@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants