(Mostly) Form Fixes #894

cchaos · 2018-06-01T19:06:21Z

1. Fixed line-height of h4’s in text

2. Updated Sketch zip version

3. Fixed switch background in case it’s been placed on a gray background

4. Form control layout icon fixes

Lessened spacing between icons
Using focus ring on buttons instead of changing color
Created a mixin for helping create the padding necessary depending on number or icons
Lightening fill color of icons when input is disabled
Making caret on disabled comboboxes disabled

5. Fix for Kibana overwriting `:focus` state on input that doesn’t start with `.eui`

This is what it was looking like in Kibana:

6. Fixed responsive widths of `EuiDescribedFormGroup`

Now:

7. Align contents in form rows that don’t have a label and aren’t normally 40px tall

Now:

8. Added `resize` prop to `EuiTextArea`

...and defaults to ‘vertical’ (only height)

9. Ensure the descenders don't get cut off in selects

10. Added some more form variables

...and shifting readOnly inputs to not have left padding unless it has an icon

In implementation it would look like this:

Now it looks like this:

11. Fixed Safari's rendering of search inputs

IE tested
Changelog

chandlerprall

The disabled icon prop for combo box is a great catch!

JS changes LGTM; nothing stands out to me in the SCSS but I'll definitely defer to @snide for those.

snide · 2018-06-01T20:42:43Z

src/components/form/form_control_layout/_mixins.scss


-  &:focus, &:hover {
-    background-color: $euiColorPrimary;
+  &:focus {


Removing the blue for the focus state is fine, but do you think we can at least provide a hover state for these? Even just a slightly darker gray would be fine.

So the reason I took out the :hover part here was to emulate the default behavior from the type="search"

I'm going to leave the no hover state until/unless we decide to replace the stock search clear with our own.

Yep. Your reasoning makes sense to me.

snide · 2018-06-01T20:44:22Z

src/components/form/form_row/_form_row.scss

+    // as inputs will align to the vertical center
+    min-height: $euiSizeXXL;
+    padding-bottom: 0;
+    justify-content: center;


This is a dope solution. 👍

snide

Spent a bunch of time testing in browser, and did some tests in the compressed stuff as well. It looks good. Left some super small comments, but this looks good for merge. Nice work.

snide · 2018-06-01T20:53:30Z

src/components/form/text_area/_text_area.scss

  // Textareas have their own sizing
-  height: auto !important;
+  &, &--compressed {


This looks fine here and is perfectly readable cuz its so small, but can we agree not to use a plain & on something larger? It makes the scss harder to troubleshoot. Where is this height coming from? It's not on the selector!...etc.

Yeah, I was just trying to save on duplication. Would adding a line-break between the two selectors help?

That will help.

snide · 2018-06-01T21:05:03Z

src/components/form/select/_select.scss

  @include euiFormControlIsLoading($isNextToIcon: true);

-  padding-right: $euiSizeXXL; /* 1 */
-  appearance: none;
+  line-height: $euiFormControlHeight; /* 2 */


TY for fixing this.

cchaos · 2018-06-04T20:40:03Z

I just now need to wait for #898 to get merged or else we'll have a conflict.

in case it’s been placed on a gray background

- Lessened spacing between icons - Using focus ring on buttons instead of changing color - Created a mixin for helping create the padding necessary depending on number or icons

:focus state on input that doesn’t start with `.eui`

and aren’t normally 40px tall

And defaults to ‘vertical’ (only height)

and shifting readOnly inputs to not have left padding unless it has an icon

…lastic#898)" This reverts commit b78e76d. # Conflicts: # CHANGELOG.md # src/components/form/file_picker/_file_picker.scss # src/components/form/form_control_layout/_mixins.scss

…Layout. (elastic#898)"" This reverts commit 7d645ee.

cchaos requested review from chandlerprall and snide June 1, 2018 19:06

chandlerprall approved these changes Jun 1, 2018

View reviewed changes

snide reviewed Jun 1, 2018

View reviewed changes

snide approved these changes Jun 1, 2018

View reviewed changes

cchaos added 15 commits June 5, 2018 09:55

Fixing line-height of h4’s in text

6a2e110

updating Sketch zip version

469cd9b

Fixed switch background

21ffb2f

in case it’s been placed on a gray background

Form control layout icon fixes

6a92022

- Lessened spacing between icons - Using focus ring on buttons instead of changing color - Created a mixin for helping create the padding necessary depending on number or icons

Fix for Kibana overwriting

6326e08

:focus state on input that doesn’t start with `.eui`

Fixed responsive widths of EuiDescribedFormGroup

eef1978

Align contents in form rows that don’t have a label

90621da

and aren’t normally 40px tall

Added resize prop to EuiTextArea

1840931

And defaults to ‘vertical’ (only height)

Ensure the descenders don't get cut off in selects

0cd400e

Making caret on disabled comboboxes disabled

ff87d23

Adding some more form variables

6b46371

and shifting readOnly inputs to not have left padding unless it has an icon

IE fix for clear icon

b353997

changelog

30f58c3

Fixed Safari’s rendering of search inputs

8213645

Added space between selectors

1bf6bc4

cchaos force-pushed the fixes branch from f4db0a4 to 1bf6bc4 Compare June 5, 2018 14:00

cchaos added 3 commits June 5, 2018 10:27

Revert "Fix focus state regression of icons in EuiFormControlLayout. (e…

7d645ee

…lastic#898)" This reverts commit b78e76d. # Conflicts: # CHANGELOG.md # src/components/form/file_picker/_file_picker.scss # src/components/form/form_control_layout/_mixins.scss

Revert "Revert "Fix focus state regression of icons in EuiFormControl…

a35448d

…Layout. (elastic#898)"" This reverts commit 7d645ee.

Fixing conflicts

8de752c

cchaos merged commit c4f7fd8 into elastic:master Jun 5, 2018

cchaos deleted the fixes branch June 5, 2018 15:55

cchaos mentioned this pull request Jun 5, 2018

Fixing form layout icon vertical alignment #904

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(Mostly) Form Fixes #894

(Mostly) Form Fixes #894

cchaos commented Jun 1, 2018 •

edited

Loading

chandlerprall left a comment

snide Jun 1, 2018

cchaos Jun 4, 2018

cchaos Jun 4, 2018

snide Jun 4, 2018

snide Jun 1, 2018

cchaos Jun 4, 2018

snide left a comment

snide Jun 1, 2018

cchaos Jun 4, 2018

snide Jun 4, 2018

snide Jun 1, 2018

cchaos Jun 4, 2018

cchaos commented Jun 4, 2018

(Mostly) Form Fixes #894

(Mostly) Form Fixes #894

Conversation

cchaos commented Jun 1, 2018 • edited Loading

1. Fixed line-height of h4’s in text

2. Updated Sketch zip version

3. Fixed switch background in case it’s been placed on a gray background

4. Form control layout icon fixes

5. Fix for Kibana overwriting :focus state on input that doesn’t start with .eui

6. Fixed responsive widths of EuiDescribedFormGroup

7. Align contents in form rows that don’t have a label and aren’t normally 40px tall

8. Added resize prop to EuiTextArea

9. Ensure the descenders don't get cut off in selects

10. Added some more form variables

11. Fixed Safari's rendering of search inputs

chandlerprall left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snide left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cchaos commented Jun 4, 2018

cchaos commented Jun 1, 2018 •

edited

Loading

5. Fix for Kibana overwriting `:focus` state on input that doesn’t start with `.eui`

6. Fixed responsive widths of `EuiDescribedFormGroup`

8. Added `resize` prop to `EuiTextArea`