-
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
Upgrade EUI to v87.1.0 #163961
Upgrade EUI to v87.1.0 #163961
Conversation
- Its new Emotion wrapper causes Enzyme to fail
860bea3
to
73b2b0a
Compare
- note: to take advantage of the new table pagination defaults, consider removing the initially set `pageSize` state
…nd DRY out types
@elasticmachine merge upstream |
@elasticmachine merge upstream |
@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.
cloud_security_posture
changes looks good
"#a6afbf", | ||
"#8c95a5", | ||
"#757c8b", | ||
"#c3c9d4", |
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.
I could not find any changes in the logs that resulting in color picker snapshot changes. Can you explain why these hex codes changed? The are generated with euiPaletteGray
.
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.
Hey Nathan! I was a little confused about this snapshot change too, but it looks like the change likely occurred due to our underlying chroma.js
dependency upgrade: elastic/eui#7039
@tkajtoch, do you mind jumping in here if you would consider these changes expected? FWIW, I'm looking at our docs' palettes before and after and I can barely detect a difference to the human eye, so it's likely not a major impact.
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.
Also, just wanted to mention our general changelog philosophy - we've historically skipped logging tech debt items / only log items that would directly impact consumer usage. We didn't expect this one to impact consumers or end-users, but the JS dependency ecosystem being what it is, an unexpected side effect emerged 😬 (although it's fortunately fairly benign in this case).
From a general developer experience perspective, I'd be curious to hear if you think it's worth logging all changes, including dependency upgrades, and whether the extra noise would be worth the extra traceability!
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.
I looked into the changes and compared updated colors. Usually, tools like chroma.js convert input colors to a broader color space before calculating color variations, specifically to have a long enough bit depth for the colors to look correct after processing. 64-bit float values in HSL color space are way more precise than 8-bit RGB. I'm 99% sure it's caused by switching to using a binomial of degree n in this commit. We see the changes in chroma.scale
because it's used here and probably in other places as well that eventually affect chroma.scale
results.
The color difference can't realistically be noticed to the naked eye and even though hue and saturation are updated for each color it just looks like some kind of gamma correction while keeping luminance the same.
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 explanation.
From a general developer experience perspective, I'd be curious to hear if you think it's worth logging all changes, including dependency upgrades, and whether the extra noise would be worth the extra traceability!
I think its worth putting all changes in the change log. So there is a single comprehensive list of differences to consult.
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 Nathan! I'll take that feedback back to the team and discuss. No promises on timelines, but agreed that it's bitten us in the past before and will potentially make a difference in the future for debugging.
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.
kibana-gis changes LGTM
code review only
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 - security-detection-engine changes. Pulled down and tested rule creation, exceptions.
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.
@elastic/security-threat-hunting-investigations changes look good. 👍
@cee-chen perhaps because of the |
@darnautov Thanks a million for catching this! Should be fixed with d3b45ad. |
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.
ML and Transform changes tested and LGTM
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, thanks!
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
v86.0.0
⏩v87.1.0
pageSize
andpageSizeOptions
now optional props forEuiBasicTable.pagination
,EuiInMemoryTable.pagination
andEuiDataGrid.pagination
.This caused several other components that were cloning EUI's pagination type to start throwing type warnings about
pageSize
being optional. Where I came across these errors, I modified the extended types to requirepageSize
. These types and their usages may end up changing again in any case once the Shared UX team looks into #56406.87.1.0
EuiAutoSizer
. This primarily affects typing around thedisableHeight
anddisableWidth
props (#6798)EuiAutoSize
,EuiAutoSizeHorizontal
, andEuiAutoSizeVertical
types to supportEuiAutoSizer
's now-stricter typing (#6798)EuiDatePickerRange
to supportcompressed
display (#7058)EuiFlyoutBody
with a newscrollableTabIndex
prop (#7061)panelMinWidth
prop toEuiInputPopover
(#7071)inputPopoverProps
prop forEuiRange
s andEuiDualRange
s withshowInput="inputWithPopover"
set (#7082)Bug fixes
EuiToolTip
overriding instead of merging itsaria-describedby
tooltip ID with any existingaria-describedby
s (#7055)EuiSuperDatePicker
'scompressed
display (#7058)EuiAccordion
to remove tabbable children from sequential keyboard navigation when the accordion is closed (#7064)EuiFlyout
s to accept customaria-describedby
IDs (#7065)Accessibility
dialog
role andtabIndex
from pushEuiFlyout
s. Push flyouts, compared to overlay flyouts, require manual accessibility management. (#7065)87.0.0
componentDefaults
prop toEuiProvider
, which will allow configuring certain default props globally. This list of components and defaults is still under consideration. (#6923)EuiPortal
'sinsert
prop can now be configured globally viaEuiProvider.componentDefaults
(#6941)EuiFocusTrap
'scrossFrame
andgapMode
props can now be configured globally viaEuiProvider.componentDefaults
(#6942)EuiTablePagination
'sitemsPerPage
,itemsPerPageOptions
, andshowPerPageOptions
props can now be configured globally viaEuiProvider.componentDefaults
(#6951)EuiBasicTable
,EuiInMemoryTable
, andEuiDataGrid
now allowpagination.pageSize
to be undefined. If undefined,pageSize
defaults toEuiTablePagination
'sitemsPerPage
component default. (#6993)EuiBasicTable
,EuiInMemoryTable
, andEuiDataGrid
'spagination.pageSizeOptions
will now fall back toEuiTablePagination
'sitemsPerPageOptions
component default. (#6993)EuiHeaderLinks
'sgutterSize
spacings (#7005)EuiHeaderAlert
's stacking styles (#7005)toolTipProps
toEuiListGroupItem
that allows customizing item tooltips. (#7018)EuiBreadcrumbs
to support breadcrumbs that toggle popovers viapopoverContent
andpopoverProps
(#7031)EuiSteps
andEuiStepsHorizontal
to meet WCAG AA guidelines. (#7032)EuiSteps
andEuiStepsHorizontal
to highlight and provide a more clear visual indication of the current step (#7048)Bug fixes
<EuiHeaderSectionItem side="right" />
now align right as expected without needing a previousside="left"
sibling. (#7005)EuiPageTemplate
now correctly displayspanelled={true}
(#7044)Breaking changes
EuiTablePagination
's defaultitemsPerPage
is now10
(was previously50
). This can be configured throughEuiProvider.componentDefaults
. (#6993)EuiTablePagination
's defaultitemsPerPageOptions
is now[10, 25, 50]
(was previously[10, 20, 50, 100]
). This can be configured throughEuiProvider.componentDefaults
. (#6993)border
prop fromEuiHeaderSectionItem
(unused since Amsterdam theme) (#7005)borders
object configuration fromEuiHeader.sections
(#7005)CSS-in-JS conversions
EuiHeaderAlert
to Emotion; Removed unused.euiHeaderAlert__dismiss
CSS (#7005)EuiHeaderSection
,EuiHeaderSectionItem
, andEuiHeaderSectionItemButton
to Emotion (#7005)EuiHeaderLinks
andEuiHeaderLink
to Emotion; Removed$euiHeaderLinksGutterSizes
Sass variables (#7005)$euiHeaderBackgroundColor
Sass variable; use$euiColorEmptyShade
instead (#7005)$euiHeaderChildSize
Sass variable; use$euiSizeXXL
instead (#7005)