-
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 v37.3.1 #109926
Upgrade EUI to v37.3.1 #109926
Conversation
// make sure to clean it up when something change | ||
// this avoids cell's styling to stick forever | ||
return () => { | ||
if (minMaxByColumnId?.[originalId]) { | ||
setCellProps({ | ||
style: { | ||
backgroundColor: undefined, | ||
color: undefined, | ||
}, | ||
}); | ||
} | ||
}; |
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.
cc @dej611 on this change: elastic/eui#4665 (comment)
I tested locally and I'm fairly sure this is still working with the hook cleanup gone:
Feel free to double check/confirm yourself of course! 🙏
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.
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.
Keeping this code here the behaviour works as expected.
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.
Super strange! Will definitely revert this for this release - hopefully we can get it figured out later. Thank you for QAing @dej611!
Pinging @elastic/eui-design (EUI) |
CI is now passing, so I went ahead and undrafted to start getting eyes/reviews from the various team CODEOWNERS affected by this upgrade. It's primarily Jest snapshot updates, with 1 slight test change for the Security team (d3ce53e), a cleanup for the Lens team (e40ebfa), and some i18n changes I'm talking through in a thread above. |
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.
uptime changes 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.
Stack management changes 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! Thank you for all these changes! 🧡
@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.
Test snapshot diffs LGTM, haven't tested locally.
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.
Presentation team changes 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.
The removed code in Lens is still required to make it work correctly the table conditional coloring as the EuiDataGrid
is not cleaning it up correctly in all cases.
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.
APM changes look good
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
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.
Tested on Safari 👍
Thanks everyone - super appreciate your reviews on this! |
* Upgrade EUI to v37.3.1 * Update i18n token mappings * Skip i18n_eui_mapping defString checks for functions * Update snapshots * Update failing Security tests with extra nodes * Remove hook cleanup now that elastic/eui#5068 is merged * [i18n PR feedback] Prefer specific token skipping over all functions skipping * Revert "Remove hook cleanup now that elastic/eui#5068 is merged" This reverts commit e40ebfa. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
* Upgrade EUI to v37.3.1 * Update i18n token mappings * Skip i18n_eui_mapping defString checks for functions * Update snapshots * Update failing Security tests with extra nodes * Remove hook cleanup now that elastic/eui#5068 is merged * [i18n PR feedback] Prefer specific token skipping over all functions skipping * Revert "Remove hook cleanup now that elastic/eui#5068 is merged" This reverts commit e40ebfa. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Constance <constancecchen@users.noreply.github.com>
Summary
eui@37.3.0
⏩eui@37.3.1
37.3.1
Bug fixes
EuiDataGrid
where a customclassName
was also being passed to the full screen button (#5050)PaginationButton
insideEuiPagination
(#5048)euiHeaderAffordForFixed
mixin that was not accounting for situations whereEuiDataGrid
was in full screen mode (#5054)z-index
styles that were causingEuiModal
andEuiFlyout
components to appear behindEuiDataGrid
when in full screen mode (#5054)EuiFilterButton
- adds 2 new tokens and removes oldeuiFilterButton.filterBadge
token (#4750)EuiFilePicker
's default prompt, and improved i18n string foreuiFilePicker.filesSelected
(#5063)EuiDataGrid
sort button text pluralization (#5043)EuiButtonIcon
when passingdisabled
prop (#5060)EuiDataGrid
not clearing cell styles when column position changes (#5068)Theme: Amsterdam
EuiDatePicker
(#5000)EuiSuperDatePicker
(#5060)