Conversation
- removed unnecessary modifier classes
- `showOnFocus` condition appended
- `-isInteractive` appended to account for new styles
…der behavior A visually hidden `, ` is now included in the selected option text for super select components with used within `<EuiFormRow>`s with labels This affects `===` text assertions, but changing them to `contains` instead if a simpler and quicker approach that keeps the meaning of the test while passing it
|
/ci |
|
Pinging @elastic/eui-team (EUI) |
|
Pinging @elastic/fleet (Team:Fleet) |
|
👋 Hey y'all! As this PR fairly minor and is 90% snapshot updates (and here is your scheduled reminder to consider moving away from snapshots!), we'll be asking KibanaOps for an admin merge (regardless of approvals) by Wednesday. We have a much larger release & upgrade PR landing very shortly that will contain EuiTable (including basic and in memory table) breaking changes and enhancements, and we'd much rather y'all spend your time reviewing and QAing that one. Stay tuned! |
gergoabraham
left a comment
There was a problem hiding this comment.
@elastic/security-defend-workflows related changes LGTM
one minor thing as a paranoid test writer: we had changes only in test files, all of them are around changing the assertion from toEqual() to toContain(), because the rendered text content now ends with , .
do you think this change could decrease trust in tests? e.g. for OSes it asserts it contains 'windows', but in the background the element contains all the other OSes as well because of e.g. a bad selector or a future change in EUI or a future bug introduced by a developer in our team.
if you're confident with these changes, I'm okay with them, otherwise I'd suggest going with toEqual('Windows, ')
mbondyra
left a comment
There was a problem hiding this comment.
Vis changes lgtm, (tested the combobox)
ElenaStoeva
left a comment
There was a problem hiding this comment.
Snapshot changes in es_ui_shared plugin LGTM
In this case with the tests as written, I'm fairly confident for two reasons: EuiSuperSelect can only ever contain 1 selection and not multiple, and the options in the test (Windows, Mac, and Linux) are different enough that the wrong item being selected will correctly trigger a failure. That being said, I totally get it if your team would prefer a more strict assertion, and that's totally valid. I'll go ahead and make that change for your team's test files now! |
tsullivan
left a comment
There was a problem hiding this comment.
AppEx-SharedUX changes LGTM
Reviewed the snapshot changes.
💚 Build Succeeded
Metrics [docs]Page load bundle
History
To update your PR or re-run it, just comment with: |
v93.5.2⏩v93.6.0v93.6.0EuiBreadcrumbstyles to improve visual distinction of clickable breadcrumbs (#7615)Deprecations
colorprop onEuiBreadcrumb(#7615)Bug fixes
EuiComboBoxto correctly select full matches within groups via theEnterkey (#7658)Accessibility
EuiHeaderBreadcrumbstyles to ensure min. required color contrast (#7643)EuiSuperSelectnow correctly reads out parentEuiFormRowlabels to screen readers (#7650)EuiSuperSelectnow more closely mimics native<select>behavior in its keyboard behavior and navigation (#7650)EuiSuperSelectno longer strands keyboard focus on close (#7650)EuiSuperSelectnow correctly allows keyboard navigating past disabled options in the middle of the options list (#7650)