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

Upgrade EUI to v84.0.0 #161716

Merged
merged 12 commits into from
Jul 14, 2023
Merged

Upgrade EUI to v84.0.0 #161716

merged 12 commits into from
Jul 14, 2023

Conversation

cee-chen
Copy link
Member

@cee-chen cee-chen commented Jul 11, 2023 •
edited
Loading

Summary

[email protected] ⏩ [email protected]

84.0.0

  • Updated EuiDualRange's minInputProps and maxInputProps to support passing more props to underlying inputs (#6902)
  • EuiFocusTrap now supports configuring cross-iframe focus trapping via the crossFrame prop (#6908)

Bug fixes

  • Fixed EuiFilterButton icon display (#6900)
  • Fixed EuiCombobox compressed plain text display (#6910)
  • Fixed visual appearance of collapse buttons on collapsible EuiResizablePanels (#6926)

Breaking changes

  • EuiFocusTrap now defaults to not trapping focus across iframes (#6908)

@cee-chen cee-chen added release_note:skip Skip the PR/issue when compiling release notes EUI labels Jul 11, 2023
@cee-chen
Fix references to removed .euiAccordion__padding classes
62eaaab
@cee-chen
Update EuiAccordion snapshots
fda31d9
@cee-chen
Update HTML snapshots
e64b8c9
@cee-chen
[Security] Remove mounted snapshots
c29f8db 
- mounted snapshots are a huge headache especially with the Emotion wrappers that don't provide meaningful information and cause snapshot churn on style changes
@cee-chen cee-chen marked this pull request as ready for review July 12, 2023 03:00
@cee-chen cee-chen requested review from a team as code owners July 12, 2023 03:00
@elasticmachine
Copy link
Contributor

Pinging @elastic/eui-team (EUI)

@cee-chen cee-chen requested a review from banderror July 12, 2023 03:00
ashokaditya
ashokaditya approved these changes Jul 12, 2023
View reviewed changes
Copy link
Member

@ashokaditya ashokaditya left a comment

Choose a reason for hiding this comment

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

Thanks for the update.

patrykkopycinski
patrykkopycinski reviewed Jul 12, 2023
View reviewed changes
yarn.lock Outdated Show resolved Hide resolved
thomheymann
thomheymann approved these changes Jul 12, 2023
View reviewed changes
Copy link
Contributor

@thomheymann thomheymann left a comment

Choose a reason for hiding this comment

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

security changes LGTM

ThomThomson
ThomThomson approved these changes Jul 12, 2023
View reviewed changes
Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Presentation snapshot changes LGTM

@cee-chen cee-chen added v8.10.0 and removed v8.9.0 labels Jul 13, 2023
Dosant
Dosant approved these changes Jul 13, 2023
View reviewed changes
Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

snapshots lgtm

@yctercero
Copy link
Contributor

Hey! Checking it out for security-detection-engine - just see one little thing where one of our components now doesn't take the full parent width. Is there something we need to update here?

Before
Screenshot 2023-07-13 at 9 20 58 AM

After
Screenshot 2023-07-13 at 9 21 05 AM

@cee-chen
Copy link
Member Author

Thanks for the thorough QA / catching that @yctercero! That's an odd change, as I'm not seeing anything in v84's changelog that would cause it. Let me pull in latest to make sure it's not related to #161592 and debug locally.

@cee-chen
Copy link
Member Author

@elasticmachine merge upstream

kibanamachine and others added 2 commits July 13, 2023 12:50
@kibanamachine
Merge branch 'main' into eui-v84.0.x
0c123af
@cee-chen
Fix Security exceptions list accordion usage of Emotion
a3c1857 
- the `width: 100%` was the main fix, but using `@emotion/react` and styled components in the same plugin does not work correctly (per the removed comment). Either migrate the whole component or use `@emotion/css` in the interim instead
@cee-chen
Copy link
Member Author

cee-chen commented Jul 13, 2023 •
edited
Loading

@yctercero It looks like an internal accordion change in this release is causing the issue - thank you again for catching it. Changing display: block to width: 100% resolves the problem (a3c1857).

While I was there, I also tweaked the component's use of Emotion - the existing comment that noted issues was due to plugins not being able to use both @emotion/react and styled-components in the same setup. See elastic/eui#6828 (comment) for more information.

yctercero
yctercero approved these changes Jul 13, 2023
View reviewed changes
Copy link
Contributor

@yctercero yctercero left a comment

Choose a reason for hiding this comment

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

LGTM - thanks so much for the fix!

banderror
banderror approved these changes Jul 13, 2023
View reviewed changes
Copy link
Contributor

@banderror banderror left a comment

Choose a reason for hiding this comment

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

Detection Rule Management changes LGTM 👍

semd
semd approved these changes Jul 14, 2023
View reviewed changes
packages/kbn-securitysolution-grouping/src/components/styles.tsx
@@ -36,7 +36,7 @@ export const statsContainerCss = css`
`;

export const groupingContainerCss = css`
.groupingAccordionForm .euiAccordion__childWrapper .euiAccordion__padding--m {
.groupingAccordionForm .euiAccordion__childWrapper .euiAccordion__children {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@cee-chen
Copy link
Member Author

@elasticmachine merge upstream

@cee-chen
Copy link
Member Author

@elasticmachine merge upstream

@cee-chen cee-chen enabled auto-merge (squash) July 14, 2023 16:56
@kibana-ci
Copy link
Collaborator

kibana-ci commented Jul 14, 2023 •
edited
Loading

💔 Build Failed

Failed CI Steps

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 9.8MB 9.8MB -59.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
kbnUiSharedDeps-css 296.4KB 296.4KB +26.0B
kbnUiSharedDeps-npmDll 6.0MB 6.0MB +1.7KB
total +1.7KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@cee-chen cee-chen merged commit fc9ac7a into elastic:main Jul 14, 2023
@cee-chen cee-chen deleted the eui-v84.0.x branch July 14, 2023 18:16
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jul 14, 2023
@cee-chen
Copy link
Member Author

Whoa, I'm not totally sure why the PR auto-merged with a failing Security Cypress test, apologies all. I assume it's a flake as the previous run passed - Security team, please feel free to ping me if not!

@Heenawter Heenawter mentioned this pull request Jul 14, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patrykkopycinski patrykkopycinski patrykkopycinski left review comments

@semd semd semd approved these changes

@tomsonpl tomsonpl tomsonpl approved these changes

@ThomThomson ThomThomson ThomThomson approved these changes

@Ikuni17 Ikuni17 Ikuni17 approved these changes

@yctercero yctercero yctercero approved these changes

@Dosant Dosant Dosant approved these changes

@banderror banderror banderror approved these changes

@mistic mistic mistic approved these changes

@ashokaditya ashokaditya ashokaditya approved these changes

@thomheymann thomheymann thomheymann approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting EUI release_note:skip Skip the PR/issue when compiling release notes v8.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.