-
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 v52.2.0 #128313
Upgrade EUI to v52.2.0 #128313
Conversation
…pshots, and updated equality conditions for specific test cases
…ing for strict text equality to account for text coming from the EuiScreenReaderOnly component. Also updated tests to account for EuiIcon text that is now rendered when the icon is imported from .testenv (PR 5709 - elastic/eui#5709).
…into upgrade-eui-52.1.0 Pulling the latest code from the upgrade-eui-52.1.0 branch
…apshots. Updated tests using getAllByLabelText and getByLabelText to getAllByText and getByText respectively as the former have been deprecated
…pshots. Updated instances of getByLabelText and getAllByLabelText to getByText and getAllByText as the former are now deprecated.
@@ -25,7 +25,7 @@ describe('NotFoundErrors component', () => { | |||
const callOut = mounted.find('EuiCallOut'); | |||
expect(callOut).toMatchSnapshot(); | |||
expect(mounted.text()).toMatchInlineSnapshot( | |||
`"There is a problem with this saved objectThe saved search associated with this object no longer exists.If you know what this error means, you can use the Saved objects APIs(opens in a new tab or window) to fix it — otherwise click the delete button above."` | |||
`"There is a problem with this saved objectThe saved search associated with this object no longer exists.If you know what this error means, you can use the Saved objects APIsExternal link(opens in a new tab or window) to fix it — otherwise click the delete button 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.
Hmm I am not sure this change from APIs
to APIsExternal link
is intended?
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 Bamieh, this change comes from PR 5709 (elastic/eui#5709) where all of the icons being used by .testenv
now render text as a <span>
to notate what the icon is. Now, where an icon would be used, we will see text for that icon inline. I'm in the process of updating the tests that were affected by 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.
OK cool. Thank you 👍
… Updated required snapshots. Updated various tests to account for text rendering of the EuiIcon text.
…age.json and license_checker/config.ts
Pinging @elastic/uptime (Team:uptime) |
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.
security_solution and timelines 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.
deployment management changes LGTM
💛 Build succeeded, but was flakyTest Failures
Metrics [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.
APM 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
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.
security 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.
Canvas 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.
VisEditors 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, just snapshot were changed, did also a very basic cloud test, works as expected
Merging ahead of the awaiting reviews at the request of the EUI team. If you have any issues here please raise them with EUI, thanks. |
This reverts commit dccd829.
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 and User Experience plugin changes LGTM. Only reviewed code as just a few tests are changed.
Friendly reminder: Looks like this PR hasn’t been backported yet. |
Summary
eui@51.1.0
⏩eui@52.2.0
EuiIcon
content including thearia-label
as text. This helps the automated a11y testing suite, but the EUI team is evaluating a different approach that would be less invasive. It's possible that these snapshot changes will be negated in the next release of EUI.52.2.0
branchUser
,desktop
andsessionViewer
glyphs toEuiIcon
(#5740)52.1.0
anchor
prop toEuiTourStep
to allow for DOM selector attachment (#5696)EuiDataGrid
now forcesisExpandable
to be true if anycellActions
are passed, as keyboard users are otherwise unable to access cell actions without the expansion popover (#5710)Bug fixes
EuiContextMenu
requiring two tab keypresses to advance to the next focusable menu item (#5719)EuiDataGrid
footer cell focus bugging out after moving its column (#5720)52.0.0
editorChecklist
glyph toEuiIcon
(#5705)testenv
mock forEuiIcon
to renderaria-label
as text (#5709)compressed
prop toEuiFilterGroup
and reduced the size of theEuiFilterButton
notification badge (#5717)EuiSelectableTemplateSitewide
input text when in dark header (#5724)Breaking changes
flush
andsize
props inEuiFilterButtonProps
(#5717)CSS-in-JS conversions
EuiMark
to CSS-in-JS styling (#4575)