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.

Sign up for GitHub

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

Jump to bottom

[EuiFlyout] Fix broken push flyout behavior #6764

Merged
merged 3 commits into from
May 9, 2023
Merged

[EuiFlyout] Fix broken push flyout behavior #6764

merged 3 commits into from
May 9, 2023

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented May 9, 2023 •
edited
Loading

Summary

#6645 and #6744 made EuiFlyouts use padding-right to preserve scrollbar width, which react-remove-scrollbar unfortunately sets !important for, which overrides the inline padding styles push flyouts use to push the page content.

The fix to this issue that push flyouts should not have scrollLock enabled - I've taken several measures at both the EuiFlyout and EuiFocusTrap levels to ensure this is the case.

QA

General checklist

breehall
breehall approved these changes May 9, 2023
View reviewed changes
Copy link
Contributor

@breehall breehall left a comment

Choose a reason for hiding this comment

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

Tested locally against main. The pushed flyout in main lays on top of the body content and in this branch, the body content is pushed to the side (as it should be). Once you finish adding your last tests, this is good to go!

@cee-chen
Harden scrollLock behavior on EuiFlyout
f494a6e 
- push flyouts should still never set scrollLock to true
@cee-chen
Copy link
Member Author

cee-chen commented May 9, 2023

Thank you so much! 🙇

@cee-chen cee-chen marked this pull request as ready for review May 9, 2023 23:08
@cee-chen cee-chen enabled auto-merge (squash) May 9, 2023 23:09
@kibanamachine
Copy link

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

@cee-chen cee-chen merged commit 5b62d9e into elastic:main May 9, 2023
@cee-chen cee-chen deleted the flyout-push-fix branch May 9, 2023 23:45
This was referenced May 10, 2023
Merged
Merged
cee-chen added a commit to elastic/kibana that referenced this pull request May 10, 2023
@cee-chen
Upgrade EUI to v77.1.3 for push flyout fix (#157232)
e8e87a7 
This backport is required to fix a bug with push EUI flyouts before v8.8
release. fixes #157180

## [`77.1.3`](https://github.com/elastic/eui/tree/v77.1.3)

**Bug fixes**

- Fixed broken push `EuiFlyout` behavior
([#6764](elastic/eui#6764))
jbudz pushed a commit to elastic/kibana that referenced this pull request May 15, 2023
@cee-chen @kibanamachine
Upgrade EUI to v79.0.1 (#156925)
b9ea4ce 
## Summary

`eui@77.2.2` ⏩ `eui@79.0.1`

🦴 The primary changes in this upgrade are around the deprecated
`EuiLoadingContent` being removed in favor of `EuiSkeletonText`.
- Most instances have been a [direct swap of
usage](327626a),
but [some replacements were a bit more
opinionated](e6ceb36)
as I saw them as potential to take advantage of `EuiSkeletonText`'s
syntactical sugar and screen reader announcements for when state
switches to loaded.

---

## [`79.0.1`](https://github.com/elastic/eui/tree/v79.0.1)

**Bug fixes**

- Fixed broken push `EuiFlyout` behavior
([#6764](elastic/eui#6764))


## [`79.0.0`](https://github.com/elastic/eui/tree/v79.0.0)

- Updated all `EuiSkeleton` components with new props that allow for
more control over screen reader live announcements:
`announceLoadingStatus`, `announceLoadedStatus`, and `ariaLiveProps`
([#6752](elastic/eui#6752))
- Improved keyboard accessibility in `EuiPageHeader` by ensuring the
right side menu items come into focus from left to right.
([#6753](elastic/eui#6753))

**Breaking changes**

- Removed deprecated `EuiLoadingContent`. Use the `EuiSkeleton`
components instead. ([#6754](elastic/eui#6754))


## [`78.0.0`](https://github.com/elastic/eui/tree/v78.0.0)

- Improved the contrast ratio of `EuiCheckbox`, `EuiRadio`, and
`EuiSwitch` in their unchecked states to meet WCAG AA guidelines.
([#6729](elastic/eui#6729))
- Added React Testing Library `*ByTestSubject` custom commands to
`within()`. RTL utilities can be imported from
`@elastic/eui/lib/test/rtl`.
([#6737](elastic/eui#6737))
- Updated `EuiAvatar` to support a new letter `casing` prop that allow
customizing text capitalization
([#6739](elastic/eui#6739))
- Updated `EuiFocusTrap` to support the `gapMode` prop configuration
(now defaults to `padding`)
([#6744](elastic/eui#6744))

**Bug fixes**

- Fixed inconsistency in `EuiSearchBar`'s AND/OR semantics between DSL
and query string generation
([#6717](elastic/eui#6717))
- Fixed `EuiFieldNumber`'s native browser validity detection causing
extra unnecessary rerenders
([#6741](elastic/eui#6741))
- 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))

**Breaking changes**

- `EuiAvatar`s with the default `user` type will now default to
capitalizing all initials in uppercase
([#6739](elastic/eui#6739))

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
jasonrhodes pushed a commit to elastic/kibana that referenced this pull request May 17, 2023
@cee-chen @kibanamachine @jasonrhodes
Upgrade EUI to v79.0.1 (#156925)
14c8acc 
## Summary

`eui@77.2.2` ⏩ `eui@79.0.1`

🦴 The primary changes in this upgrade are around the deprecated
`EuiLoadingContent` being removed in favor of `EuiSkeletonText`.
- Most instances have been a [direct swap of
usage](327626a),
but [some replacements were a bit more
opinionated](e6ceb36)
as I saw them as potential to take advantage of `EuiSkeletonText`'s
syntactical sugar and screen reader announcements for when state
switches to loaded.

---

## [`79.0.1`](https://github.com/elastic/eui/tree/v79.0.1)

**Bug fixes**

- Fixed broken push `EuiFlyout` behavior
([#6764](elastic/eui#6764))


## [`79.0.0`](https://github.com/elastic/eui/tree/v79.0.0)

- Updated all `EuiSkeleton` components with new props that allow for
more control over screen reader live announcements:
`announceLoadingStatus`, `announceLoadedStatus`, and `ariaLiveProps`
([#6752](elastic/eui#6752))
- Improved keyboard accessibility in `EuiPageHeader` by ensuring the
right side menu items come into focus from left to right.
([#6753](elastic/eui#6753))

**Breaking changes**

- Removed deprecated `EuiLoadingContent`. Use the `EuiSkeleton`
components instead. ([#6754](elastic/eui#6754))


## [`78.0.0`](https://github.com/elastic/eui/tree/v78.0.0)

- Improved the contrast ratio of `EuiCheckbox`, `EuiRadio`, and
`EuiSwitch` in their unchecked states to meet WCAG AA guidelines.
([#6729](elastic/eui#6729))
- Added React Testing Library `*ByTestSubject` custom commands to
`within()`. RTL utilities can be imported from
`@elastic/eui/lib/test/rtl`.
([#6737](elastic/eui#6737))
- Updated `EuiAvatar` to support a new letter `casing` prop that allow
customizing text capitalization
([#6739](elastic/eui#6739))
- Updated `EuiFocusTrap` to support the `gapMode` prop configuration
(now defaults to `padding`)
([#6744](elastic/eui#6744))

**Bug fixes**

- Fixed inconsistency in `EuiSearchBar`'s AND/OR semantics between DSL
and query string generation
([#6717](elastic/eui#6717))
- Fixed `EuiFieldNumber`'s native browser validity detection causing
extra unnecessary rerenders
([#6741](elastic/eui#6741))
- 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))

**Breaking changes**

- `EuiAvatar`s with the default `user` type will now default to
capitalizing all initials in uppercase
([#6739](elastic/eui#6739))

---------

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@breehall breehall breehall approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cee-chen @kibanamachine @breehall