Merged
Conversation
cee-chen
added
release_note:skip
cee-chen
force-pushed
the
eui/v95.8.0
branch
4 times, most recently
from
882061a to
2d169cb
Compare
1 task
- caused by additional Emotion wrapper that matches the same selector
…ration - default props are no longer read/rendered by enzyme's snapshotter
…to `useGeneratedHtmlId` - requires updating mocks
- checkbox and radio inputs should now be directly clickable, and have a different DOM wrapper
- use EuiTitle size and font weight instead
- `.spcShareToSpaceIncludeRelated` isn't actually used anywhere in Kibana
- EuiCheckbox DOM has changed since, color: inherit should work for both disabled and non-disabled checkboxes
…ons with components instead - in the cast of the `prepend` prop, `<EuiFormLabel>` is already automatically used if the prepend type is a string
- EuiFormControlLayout should automatically handle styling, and prepend/append nodes shouldn't be used outside of form layouts
…Group`s - EuiDescribedFormGroups already automatically accounts for form rows without labels, so the `hasEmptyLabelSpace` prop is now unnecessary and adds extra unwanted spacing
Contributor
peteharverson
left a comment
peteharverson
left a comment
There was a problem hiding this comment.
For ML, spotted one regression.
x-pack/plugins/aiops/public/components/log_rate_analysis/item_filter_popover.tsx
Show resolved
Hide resolved
mbondyra
approved these changes
Aug 28, 2024
Contributor
mbondyra
left a comment
mbondyra
left a comment
There was a problem hiding this comment.
Visualizations team changes LGTM! Thank you!
- doesn't apply and isn't necessary, removing it gets the button group back to the previous prod rendering
Heenawter
approved these changes
Aug 28, 2024
Contributor
There was a problem hiding this comment.
I noticed that the border on the controls is noticeably lighter in this PR than on main:
-
Before:
-
After:
Not a big deal, but is this an intentional change?
Either way, approving to unblock, since this is pretty minor - thanks so much for your effort in cleaning up our styling, it is very, very much appreciated 🙇 The new range slider styling is so clean 🤌
Contributor
Author
This is an intentional change! Borders for inputs with prepend/appends (aka grouped inputs) were previously darker than inputs without them. This is now fixed so there is no change in border lightness/darkness between grouped inputs and non-grouped inputs. |
jcger
approved these changes
Aug 29, 2024
peteharverson
approved these changes
Aug 29, 2024
Contributor
peteharverson
left a comment
peteharverson
left a comment
There was a problem hiding this comment.
LGTM for ML. Thanks for fixing the Smart grouping field!
Samiul-TheSoccerFan
approved these changes
Aug 29, 2024
Contributor
Samiul-TheSoccerFan
left a comment
Samiul-TheSoccerFan
left a comment
There was a problem hiding this comment.
LGTM, code review only.
Only one file has been updated x-pack/plugins/enterprise_search/public/applications/workplace_search/views/content_sources/components/synchronization/blocked_window_item.test.tsx
This was referenced Aug 30, 2024
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
This was referenced Sep 3, 2024
Merged
4 tasks
Ikuni17
pushed a commit
that referenced
this pull request
Sep 24, 2024
## Summary This is a follow up to EUI's Emotion conversion of **EuiFormControlLayout/Delimited** (see #190752, elastic/eui#7954, and elastic/eui#7957). > [!note] > Please manually QA your team's affected form control(s) to confirm they still look and behave as expected and are non-broken. The EUI team is not familiar enough with each plugin's setups to pull down and QA this PR ourselves. While QA testing the upgrade, I noticed a few incorrect usages of **EuiFormControlLayout** but wanted to wait until after the upgrade to push out fixes (to prevent delaying the PR further). In general, here is EUI's [recommended usage of the component](https://eui.elastic.co/#/forms/form-controls#form-control-layout): - Where possible, **simply don't use it**. Almost all form controls are **already** automatically wrapped in any EuiFormControlLayout by default, and should accept a large majority of the props that the layout accepts. - If you **must** use it, set the `controlOnly` prop on the child input/control to avoid buggy styling (e.g. duplicate borders). - If you can't do either of the above for any reason (e.g. missing prop support), reach out to the EUI team to ask for your UX as a feature request! ### Checklist Delete any items that are not applicable to this PR. - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers)
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Sep 24, 2024
## Summary This is a follow up to EUI's Emotion conversion of **EuiFormControlLayout/Delimited** (see elastic#190752, elastic/eui#7954, and elastic/eui#7957). > [!note] > Please manually QA your team's affected form control(s) to confirm they still look and behave as expected and are non-broken. The EUI team is not familiar enough with each plugin's setups to pull down and QA this PR ourselves. While QA testing the upgrade, I noticed a few incorrect usages of **EuiFormControlLayout** but wanted to wait until after the upgrade to push out fixes (to prevent delaying the PR further). In general, here is EUI's [recommended usage of the component](https://eui.elastic.co/#/forms/form-controls#form-control-layout): - Where possible, **simply don't use it**. Almost all form controls are **already** automatically wrapped in any EuiFormControlLayout by default, and should accept a large majority of the props that the layout accepts. - If you **must** use it, set the `controlOnly` prop on the child input/control to avoid buggy styling (e.g. duplicate borders). - If you can't do either of the above for any reason (e.g. missing prop support), reach out to the EUI team to ask for your UX as a feature request! ### Checklist Delete any items that are not applicable to this PR. - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) (cherry picked from commit fd7b86e)
kibanamachine
added a commit
that referenced
this pull request
Sep 24, 2024
# Backport This will backport the following commits from `main` to `8.x`: - [Fix various EuiFormControlLayout usages (#192779)](#192779) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Cee Chen","email":"549407+cee-chen@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-09-24T20:55:59Z","message":"Fix various EuiFormControlLayout usages (#192779)\n\n## Summary\r\n\r\nThis is a follow up to EUI's Emotion conversion of\r\n**EuiFormControlLayout/Delimited** (see\r\nhttps://github.com//pull/190752,\r\nhttps://github.com/elastic/eui/pull/7954, and\r\nhttps://github.com/elastic/eui/pull/7957).\r\n\r\n> [!note]\r\n> Please manually QA your team's affected form control(s) to confirm\r\nthey still look and behave as expected and are non-broken. The EUI team\r\nis not familiar enough with each plugin's setups to pull down and QA\r\nthis PR ourselves.\r\n\r\nWhile QA testing the upgrade, I noticed a few incorrect usages of\r\n**EuiFormControlLayout** but wanted to wait until after the upgrade to\r\npush out fixes (to prevent delaying the PR further). In general, here is\r\nEUI's [recommended usage of the\r\ncomponent](https://eui.elastic.co/#/forms/form-controls#form-control-layout):\r\n\r\n- Where possible, **simply don't use it**. Almost all form controls are\r\n**already** automatically wrapped in any EuiFormControlLayout by\r\ndefault, and should accept a large majority of the props that the layout\r\naccepts.\r\n- If you **must** use it, set the `controlOnly` prop on the child\r\ninput/control to avoid buggy styling (e.g. duplicate borders).\r\n- If you can't do either of the above for any reason (e.g. missing prop\r\nsupport), reach out to the EUI team to ask for your UX as a feature\r\nrequest!\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\r\n\r\n- [x] Any UI touched in this PR is usable by keyboard only (learn more\r\nabout [keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n- [x] Any UI touched in this PR does not create any new axe failures\r\n(run axe in browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n- [x] This renders correctly on smaller devices using a responsive\r\nlayout. (You can test this [in your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)","sha":"fd7b86e209e7133f3d9b7bd4e9fd6542f8a3aaad","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","EUI","v9.0.0","backport:prev-minor","ci:project-deploy-observability","Team:obs-ux-infra_services","Team:obs-ux-management","apm:review","v8.16.0"],"title":"Fix various EuiFormControlLayout usages","number":192779,"url":"https://github.com/elastic/kibana/pull/192779","mergeCommit":{"message":"Fix various EuiFormControlLayout usages (#192779)\n\n## Summary\r\n\r\nThis is a follow up to EUI's Emotion conversion of\r\n**EuiFormControlLayout/Delimited** (see\r\nhttps://github.com//pull/190752,\r\nhttps://github.com/elastic/eui/pull/7954, and\r\nhttps://github.com/elastic/eui/pull/7957).\r\n\r\n> [!note]\r\n> Please manually QA your team's affected form control(s) to confirm\r\nthey still look and behave as expected and are non-broken. The EUI team\r\nis not familiar enough with each plugin's setups to pull down and QA\r\nthis PR ourselves.\r\n\r\nWhile QA testing the upgrade, I noticed a few incorrect usages of\r\n**EuiFormControlLayout** but wanted to wait until after the upgrade to\r\npush out fixes (to prevent delaying the PR further). In general, here is\r\nEUI's [recommended usage of the\r\ncomponent](https://eui.elastic.co/#/forms/form-controls#form-control-layout):\r\n\r\n- Where possible, **simply don't use it**. Almost all form controls are\r\n**already** automatically wrapped in any EuiFormControlLayout by\r\ndefault, and should accept a large majority of the props that the layout\r\naccepts.\r\n- If you **must** use it, set the `controlOnly` prop on the child\r\ninput/control to avoid buggy styling (e.g. duplicate borders).\r\n- If you can't do either of the above for any reason (e.g. missing prop\r\nsupport), reach out to the EUI team to ask for your UX as a feature\r\nrequest!\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\r\n\r\n- [x] Any UI touched in this PR is usable by keyboard only (learn more\r\nabout [keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n- [x] Any UI touched in this PR does not create any new axe failures\r\n(run axe in browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n- [x] This renders correctly on smaller devices using a responsive\r\nlayout. (You can test this [in your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)","sha":"fd7b86e209e7133f3d9b7bd4e9fd6542f8a3aaad"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192779","number":192779,"mergeCommit":{"message":"Fix various EuiFormControlLayout usages (#192779)\n\n## Summary\r\n\r\nThis is a follow up to EUI's Emotion conversion of\r\n**EuiFormControlLayout/Delimited** (see\r\nhttps://github.com//pull/190752,\r\nhttps://github.com/elastic/eui/pull/7954, and\r\nhttps://github.com/elastic/eui/pull/7957).\r\n\r\n> [!note]\r\n> Please manually QA your team's affected form control(s) to confirm\r\nthey still look and behave as expected and are non-broken. The EUI team\r\nis not familiar enough with each plugin's setups to pull down and QA\r\nthis PR ourselves.\r\n\r\nWhile QA testing the upgrade, I noticed a few incorrect usages of\r\n**EuiFormControlLayout** but wanted to wait until after the upgrade to\r\npush out fixes (to prevent delaying the PR further). In general, here is\r\nEUI's [recommended usage of the\r\ncomponent](https://eui.elastic.co/#/forms/form-controls#form-control-layout):\r\n\r\n- Where possible, **simply don't use it**. Almost all form controls are\r\n**already** automatically wrapped in any EuiFormControlLayout by\r\ndefault, and should accept a large majority of the props that the layout\r\naccepts.\r\n- If you **must** use it, set the `controlOnly` prop on the child\r\ninput/control to avoid buggy styling (e.g. duplicate borders).\r\n- If you can't do either of the above for any reason (e.g. missing prop\r\nsupport), reach out to the EUI team to ask for your UX as a feature\r\nrequest!\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\r\n\r\n- [x] Any UI touched in this PR is usable by keyboard only (learn more\r\nabout [keyboard accessibility](https://webaim.org/techniques/keyboard/))\r\n- [x] Any UI touched in this PR does not create any new axe failures\r\n(run axe in browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n- [x] This renders correctly on smaller devices using a responsive\r\nlayout. (You can test this [in your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)","sha":"fd7b86e209e7133f3d9b7bd4e9fd6542f8a3aaad"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Cee Chen <549407+cee-chen@users.noreply.github.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
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.
v95.7.0⏩v95.9.0Caution
This PR contains the final set of Emotion conversions for all EuiForm components.⚠️ make sure to QA your UI to ensure no visual regressions have occurred! ⚠️
If your plugin contains any custom CSS/styling to EuiFormRow, EuiFormControlLayout, EuiCheckbox, EuiRadio, or EuiSwitch,
v95.9.0EuiSearchBar's optionalbox.schemaprop with a newrecognizedFieldsconfiguration. This allows specifying the phrases that will be parsed as field clauses (#7960)EuiIconwith a newtokenSemanticTextglyph (#7971)Bug fixes
EuiSelectableTemplateSitewidestyles when used within a dark-themedEuiHeader(#7977)v95.8.0EuiHeaderLinks's mobile menu to set a slight popover padding by default (#7961)popoverProps.panelPaddingSizeif needed.EuiHeaderLinkto default to a size ofs(down fromm) (#7961)Accessibility
aria-labelattribute for theEuiFieldSearchclear button (#7970)Bug fixes
<EuiDualRange showInput="inputWithPopover" />form controls (#7957)Deprecations
EuiFormRow'scolumnCompressedSwitchdisplay prop. UsecolumnCompressedinstead, which will automatically account for childEuiSwitches (#7968)EuiFormRow'srowCompresseddisplay prop. Userowinstead for vertical forms, orcenterCompressedfor inline forms (#7968)EuiFormRow'shasEmptySpaceLabelprop to no longer attempt to automatically align its content to a vertical center. Use thedisplay="center"prop for that instead (#7968)CSS-in-JS conversions
EuiFormControlLayoutto Emotion (#7954).euiFormControlLayout--*iconsclassNames and--eui-form-control-layout-icons-paddingCSS var. Use--euiFormControlRightIconsCountor--euiFormControlLeftIconsCountinsteadEuiFormLayoutDelimitedto Emotion (#7957)cloneElementWithCssthrowing an error when used multiple times without akeyprop (#7957)cloneElementWithCssutility to support a third argument that allows prepending vs. appending the cloned Emotion css className (#7957)@euiFormControlLayoutClearIconSass mixin (#7959)EuiDescribedFormGroupto Emotion (#7964)EuiForm,EuiFormHelpText, andEuiFormErrorTextto Emotion (#7966)EuiFormLabelandEuiFormLegendto Emotion; Removed@euiFormLabelmixin (#7967)EuiFormRowto Emotion (#7968)EuiCheckboxto Emotion (#7969)EuiRadioto Emotion (#7969)EuiSwitchto Emotion (#7969)$euiFormCustomControlDisabledIconColor$euiFormCustomControlBorderColor$euiRadioSize$euiCheckBoxSize$euiCheckboxBorderRadius$euiSwitchHeight(and compressed/mini variants)$euiSwitchWidth(and compressed/mini variants)$euiSwitchThumbSize(and compressed/mini variants)$euiSwitchIconHeight$euiSwitchOffColoreuiIconBackgroundeuiCustomControleuiCustomControlSelectedeuiCustomControlDisabledeuiCustomControlFocused