-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Upgrade EUI to v70.2.4 #144845
Merged
Merged
Upgrade EUI to v70.2.4 #144845
Changes from 33 commits
Commits
Show all changes
59 commits
Select commit
Hold shift + click to select a range
26a094a
Upgrade EUI to v70.2.0
cee-chen 9d0a49f
Update i18n tokens
cee-chen 4047448
Address EUI tooltip TODO from previous EUI upgrade
cee-chen 44ab049
Address EUI TODO for selectable & horizontal EuiCards
cee-chen 902fa62
[EuiCode][EuiCodeBlock] Account for removed Sass variables
cee-chen b052461
[EuiCodeBlock] Account for removed niche color var
cee-chen ee12916
[EuiCodeBlock] Fix index management component using now-removed CSS c…
cee-chen 0704e41
[EuiFlex] Fix type issues
cee-chen d6351c5
[EuiFlex] Fix removed Sass gutter variables
cee-chen c52d42e
[EuiFlex] Fix CSS references to removed modifier classes
cee-chen 4a427fe
[EuiFlex] Fix tests assertions
cee-chen 681f989
[EuiFlex] Snapshot updates
cee-chen ef4eb55
[EuiListGroup][EuiCollapsibleNav] Replace removed `color="ghost"` wit…
cee-chen 237341b
[EuiListGroup] Update snapshots
cee-chen 212b25c
[EuiSkipLink] Fix event type
cee-chen 03def98
[EuiSkipLink] Fix custom usage for table
cee-chen dd64ea2
[EuiForm] Update snapshots caused by `fullWidth` support
cee-chen e6c252d
[EuiCard] Update snapshots
cee-chen 3e5cabc
[EuiSwitch][EuiCheckbox] Update snapshots
cee-chen 9fd86b5
[EuiOverlayMask] Update snapshots
cee-chen d006461
[EuiTab] Fix selectors/test assertions
cee-chen 9547e1a
[EuiBadge] Update snapshots/tests
cee-chen ab980d0
[EuiDataGrid] Update fixed `data-test-subj` selector
cee-chen 6e48660
Fix multiple tests to use rendered vs mounted snapshots
cee-chen 5265375
Update visual diff tolerance
cee-chen 1f32026
[FTR] Fix now-invalid selector caused by Emotion classes
cee-chen d43735e
[Security] Fix failing scroll test
cee-chen f92bf06
[Security] Fix broken looking flyout tabs
cee-chen b3e4cc9
[Security] Fix spacing above flyout tabs
cee-chen 3c2e62b
[Discover] Fix broken tabs/badge alignment
cee-chen 0d293fd
[Security] Fix hidden flyout/failing Cypress test caused by `styled-c…
cee-chen b6cca3f
[EuiFlex] Replace removed fractions variables
cee-chen fb6642a
[osquery] attempt to fix failing tests
cee-chen 92786f2
[APM][PR feedback] Fix size conversion var
constancecchen aa16cbc
Merge remote-tracking branch 'upstream/main' into eui-70.2.x
cee-chen 2dde3c1
[Security] Fix incorrect use of EuiFlexGrid
cee-chen 93c278a
Upgrade EUI to 70.2.1 to get backport fixes
cee-chen 14854a5
Merge remote-tracking branch 'upstream/main' into eui-70.2.x
cee-chen 6dcdf94
[Security][Cases] Fix padding previously reliant on EuiFlex margins
cee-chen 9ca6ed5
[Lens] Restore EuiListGroup display back to previous custom columns
cee-chen ca332d2
[ML][DataVisualizer] Fix incorrect usage of EuiFlexGrid
cee-chen beeb9b7
Merge remote-tracking branch 'upstream/main' into eui-70.2.x
cee-chen fb3056a
[ML][Jobs] Fix missing margins in Jobs Groups list
cee-chen d09aab4
Update snapshots from main
cee-chen 32b2384
more snapshots
cee-chen 46cccd3
Merge remote-tracking branch 'upstream/main' into eui-70.2.x
cee-chen ed28359
Update EUI to v70.2.2 to get latest backport fixes
cee-chen f8aa870
[EuiFlyout] snapshot updates
cee-chen d18cf7a
[EuiButtonDisplay] snapshot updates
cee-chen 4936913
[Maps] Fix broken facet button display
cee-chen c24c836
[Maps] snapshots
cee-chen b6f322a
Fixing `Lens` dimension editor operation button size
elizabetdev 12cba12
Upgrade EUI to v70.2.4
cee-chen c085c5e
[Advanced Settings] Fix flex gap/margins
cee-chen 0756d11
Merge remote-tracking branch 'upstream/main' into eui-70.2.x
cee-chen 759e0f8
Merge branch 'main' into eui-70.2.x
kibanamachine d5f93d3
[Security] EuiDescriptionList tweaks
cee-chen 99391c9
Merge remote-tracking branch 'upstream/main' into eui-70.2.x
cee-chen fdcfd91
Merge branch 'main' into eui-70.2.x
kibanamachine File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
138 changes: 69 additions & 69 deletions
138
...me/core-chrome-browser-internal/src/ui/header/__snapshots__/collapsible_nav.test.tsx.snap
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains 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
30 changes: 27 additions & 3 deletions
30
packages/core/i18n/core-i18n-browser-internal/src/__snapshots__/i18n_service.test.tsx.snap
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains 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
Oops, something went wrong.
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.
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.
We're already providing the theme provider at the root level of the react tree, so why do you need to add another one here? Forcing colorMode to
dark
seems wrong to me?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.
It's done because these types of sections in the
EuiCollapsibleNav
are always dark. You can see an example on our docs here: https://elastic.github.io/eui/#/navigation/collapsible-nav#nav-groups-with-lists-and-other-contentThe reason why switching/forcing the colorMode is superior because it cascades to all child components - links, text color, buttons, icon fills, etc. - everything just switches without needing a bunch of custom cascading/nesting CSS.
EuiBottomBar
also has the same logic / nested dark provider as this.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.
We also dropped support for
EuiListGroup[color=ghost]
as a design decision,(from the EUI changelog)
(from the EUI pr description)
and our upgrade strategy for Kibana historically defaults to maintain the same look & feel unless we've pre-coordinated changes with affected teams, which is why we're adding the theme provider here instead of changing the UX away from the ghost color.
FYI this is prompting a wider discussion about these types of breaking changes in elastic/eui#6364
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.
@miukimiu mentioned this as well in our sync today, but I would be curious if people would feel less confused about this change if we preserved
color="ghost"
as a prop/API option and had the component automatically internally wrap<EuiThemeProvider colorMode="dark">
instead of consumers explicitly having to.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.
Thanks for the explanations