-
Notifications
You must be signed in to change notification settings - Fork 861
[Inputs] Implement form element append/prepend design updates
#9305
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
[Inputs] Implement form element append/prepend design updates
#9305
Conversation
ad10709 to
668534a
Compare
- updates append/prepend styling, border radius, background colors
…ton from the form layout context
… color on valid state & cleanup
- base and filled buttons don't have a transition anymore either, it seems logical to remove and align
- mainly due to the added wrapper element on append/prepend
668534a to
c275d28
Compare
- the entire layout wrapper has a background
- that's not the final proposal yet, just an update
68418c9 to
a7bbf0d
Compare
|
This PR contains breaking changes. The opener of this pull request is asked to perform the following due diligence steps below, to assist EUI in our next Kibana upgrade:
|
- we'll use the custom style version going forward
…pend - there might be use cases where custom content purposefully looks different; we instead want to ensure expected usage through guidance and patterns
|
A couple of mid-review comments, mostly so you know I'm at it:
|
@acstll Woops, that's a mistake, it should use |
acstll
left a comment
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.
Code-wise it looks correct, only a single comment which is a nit!
I followed the guide for the QA — thanks a lot for providing it!
I found 3 things design-wise worth pointing out (I left a brief comment in the code where I thought it might be related but it could be wrong 😛)
(1) padding in FormControlLayout's side
I think the 4px padding in the "side" in the FormControlLayout is not present in the spec when there's a label, only when there's a button
| code | spec |
|---|---|
![]() |
![]() |
(2) gap in prepend/append
this seems to be 8px and not 4px in the spec, otherwise items are too close to each other when there's no label
| code | spec |
|---|---|
![]() |
![]() |
(3) font size for buttons
in the spec, font size for buttons seems larger (~2px) than labels only, I wonder if that should be the case for element="button"
| code | spec |
|---|---|
![]() |
![]() |
packages/eui/src/components/form/form_control_layout/form_control_layout.styles.ts
Show resolved
Hide resolved
...ges/eui/src/components/form/form_control_layout/append_prepend/form_append_prepend.styles.ts
Outdated
Show resolved
Hide resolved
packages/eui/src/components/form/form_control_layout/form_control_layout.styles.ts
Show resolved
Hide resolved
packages/eui/src/components/form/form_control_layout/form_control_layout.styles.ts
Outdated
Show resolved
Hide resolved
- additionally adjusts the spacing around form labels for visual balance as those already have their own spacing
…xpectedly - prevents e.g. EuiCheckbox label from being displayed as disabled if the form layout is disabled but the checkbox is not
- we don't want the badge being automatically adjusted on parents disabled state
@acstll We aligned with @JoseLuisGJ that we keep the matching |
- due to spacing changes
|
@acstll I added a couple small additional updates to this PR that I noticed while checking Kibana usages:
|
acstll
left a comment
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 updates @mgadewoll! I checked the ones from my comments and the other ones you described (#9305 (comment)).
All changes look good and the outcome is correct.
I was about to approve but there is this one suggestion about the label padding I think is worth sharing. Let me know what you think!
packages/eui/src/components/form/form_control_layout/form_control_layout.styles.ts
Outdated
Show resolved
Hide resolved
...ges/eui/src/components/form/form_control_layout/append_prepend/form_append_prepend.styles.ts
Outdated
Show resolved
Hide resolved
23bf918 to
9a55d71
Compare
acstll
left a comment
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 a lot for that last one update!
💚 Build SucceededHistory
cc @mgadewoll |
💚 Build Succeeded
History
cc @mgadewoll |
6a2422c
into
elastic:feat/visual-refresh-append-prepend-updates










Summary
Important
This PR merges into a feature branch.
closes https://github.com/elastic/eui-private/issues/496
Changes
EuiFormAppend/EuiFormPrependstyling:EuiButtonEmptywith variantprimarygapvalue fromxstosEuiFormAppend/EuiFormPrependto inheritisDisabledstate fromEuiFormControlLayoutEuiFormControlLayoutstyling:euiTheme.border.radius.smallinstead of0now)EuiFormControlButtonto inheritisDisabled,readOnlyandisInvalidstates fromEuiFormControlLayouticonSideprop onEuiDatePickerRangeEuiSuperDatePickerstyling:checkCircleicon on valid input dataEuiButtonEmpty(other button variants don't have a transition anymore either)Breaking changes
components.superDatePickerBackgroundSucceestokenWhy are we making this change?
✨ UI modernization: The design updates are part of the global UI modernization efforts.
Screenshots #
EuiFormControlLayoutappend/prepend - New API (usingEuiFormAppend/EuiFormPrepend)EuiFormControlLayoutappend/prepend - Custom (old) API (custom content)EuiFormControlLayoutDelimitedEuiFormControlButtoninsideEuiFormControlLayoutAutofill styles (Chrome)
EuiSuperDatePicker valid styles
EuiDatePickerRange
iconSide="right"+iconTypeImpact to users
components.superDatePickerBackgroundSucceestokenℹ️ The token is not in use but e.g. Kibana requires snapshot updates:
x-pack/platform/plugins/shared/security/public/authentication/login/components/login_form/__snapshots__/login_form.test.tsx.snapx-pack/platform/plugins/shared/security/public/management/roles/edit_role/collapsible_panel/__snapshots__/collapsible_panel.test.tsx.snapx-pack/platform/plugins/shared/security/public/management/roles/edit_role/privileges/kibana/simple_privilege_section/__snapshots__/simple_privilege_section.test.tsx.snapQA
Review stories:
EuiFormAppend(PR, feature branch)EuiFormPrepend(PR, feature branch)EuiFormControlLayoutappend/prependwith old API to compare with current production (PR, production)EuiFormControlLayoutappend/prependwith new API (PR, feature branch)EuiFormControlLayoutappend/prependwith new & old API side by side (PR, feature branch)EuiFormControlLayoutwithEuiFormControlButton(PR)EuiFormControlLayoutDelimited(PR, feature branch, production)EuiSuperDatePicker(PR, feature branch, production)Review checks:
append/prependandEuiFormAppend/EuiFormPrependmatch design specsEuiFormControlLayoutEuiFormControlLayoutDelimitedEuiFieldTextandEuiFieldNumberEuiSuperDatePickerandEuiDatePickerRangeEuiSuperDatePickerrenders an icon and no background when a valid time span is selectedGeneral checklist
@defaultif default values are missing) and playground toggles