[Emotion] Convert `EuiFieldText` #7770

cee-chen · 2024-05-17T21:53:49Z

Summary

This PR converts the EuiFieldText component to Emotion + all the form mixins required for its styles, and updates several downstream components that dogfood EuiFieldText (EuiColorPicker and EuiDatePicker primarily) to look/work as before.

This PR is merging into a feature branch. I want to do way more cleanup, particularly around EuiFormControlLayout, and I suspect there's a lot more DRYing out to be done - but this PR is already getting pretty unwieldy (lots of moving parts, small tweaks), so I picked a spot to stop once everything looked mostly non-broken 😅

As always, I strongly recommend following along by commit, but here's a high level TL;DR of changes:

EuiFieldText has been converted to Emotion, which requires converting multiple Sass mixins to JS equivalents
Removed euiFormControlSize() JS mixin in favor of a new euiFormControlStyles(), which outputs an object of multiple modifiers/states for reuse
Icon padding affordance is now handled via CSS variables (--euiFormControlLeftIconsCount and --euiFormControlRightIconsCount respectively) set dynamically via inline styles, rather than static classNames
Components that were dogfooding EuiFieldText and reusing its classNames for styling had to be updated. For example, EuiDatePicker was updated to dogfood the EuiFieldText component directly instead of an <input type="text" />

QA

Confirm all the display toggles on https://eui.elastic.co/pr_7770/#/forms/form-controls#text-field look and work as expected / the same as production
Confirm EuiColorPicker and its toggles look the same as production
Confirm EuiDatePicker and its toggles look the same as production
Confirm EuiDatePickerRange and its toggles look the same as production
Confirm EuiMarkdownEditor looks the same as production

General checklist

Browser QA
- Checked in both light and dark modes
- Checked in mobile
- Checked in Chrome, Safari, Edge, and Firefox
  ~~- [ ] Checked for accessibility including keyboard-only and screenreader modes~~
Docs site QA - N/A
Code quality checklist
- Updated Jest tests for style mixins
Release checklist
- A changelog entry exists and is marked appropriately.
Designer checklist - N/A, should not have changed, although some form color variables might need to be checked in Figma

- this wasn't migrated correctly 20 months ago, `0.05` = `2%`

JasonStoltz · 2024-05-20T16:54:27Z

Hey @cee-chen.

For b03db7f, I'm trying to understand what this is actually doing. What is the before and after? Does this impact all form components, and in what way?

cee-chen · 2024-05-20T17:27:07Z

I actually just noticed while experimenting with autofill colors for a future PR that that change is slightly broken on dark mode, so please hold as I force push up a quick change! I'll explain a step by step of the changes made as well here in a second

JasonStoltz

Hey @cee-chen.

I just had a couple of questions. I'm mainly trying to understand what is visually or functionally different before and after this PR.

I think it would be useful to enumerate those explicitly in the PR description, rather than trying to infer them based on the commit log. For instance:

Background colors for disabled states is now a slightly different shade of gray
Border colors changed from X to Y
Fixed a bug in invalid/compact date picker states. Before it was X and it has been updated to Y.

I'm less concerned about the refactors and cleanup, I glanced at those but didn't fully grok all of them. For those, I trust your judgement and and more concerned about visual regressions, so I focused on QA.

packages/eui/src/components/form/form.styles.ts

packages/eui/src/components/date_picker/date_picker_range.styles.ts

packages/eui/changelogs/upcoming/7770.md

cee-chen · 2024-05-20T18:07:14Z

I think it would be useful to enumerate those explicitly in the PR description, rather than trying to infer them based on the commit log. For instance:
Background colors for disabled states is now a slightly different shade of gray
Border colors changed from X to Y
Fixed a bug in invalid/compact date picker states. Before it was X and it has been updated to Y.

There shouldn't be any of the above kind of changes; they would have been noted in the final changelog if so. Reviewing by commit is always optional if refactors are not a primary concern, simply completing the QA steps are more than sufficient.

The border color should not have materially changed (to the visual eye) from production, although that is the most complicated set of "changes" as Elizabet already pseudo-changed it when she set up euiFormVariables a long time ago, so this was more of a change from a change. I'll dive into that per your request above in a separate thread.

packages/eui/src/components/form/form.styles.ts

JasonStoltz

@cee-chen Got it. I think I misunderstood some of your commit messages. I'm good with these changes.

cee-chen · 2024-05-20T19:30:51Z

packages/eui/src/components/form/form.styles.ts

-    borderColor: transparentize(euiTheme.border.color, 0.9),
-    borderDisabledColor: transparentize(euiTheme.border.color, 0.9),
+    borderColor: transparentize(
+      colorMode === 'DARK'
+        ? euiTheme.colors.ghost
+        : darken(euiTheme.border.color, 4),
+      0.1
+    ),


@JasonStoltz Okay, sorry about all the force pushes! To address some of your previous unthreaded comments:

For 837a3ef I'm trying to understand what this is actually doing. What is the before and after? Does this impact all form components, and in what way?

borderDisabledColor was not meaningfully being used anywhere and was the exact same color as borderColor so I removed it. It's possibly an artifact of the old theme and I don't think it needs to be kept around, as it confuses code and is an extra color function that takes time to calculate at runtime.

transparentize(euiTheme.border.color, 0.9) was, I believe, both an incorrect transparentize level and an an opinionated change Elizabet made compared to the old Sass color function:

eui/packages/eui/src/global_styling/variables/_form.scss

Lines 32 to 33 in 5c40315

$euiFormBorderOpaqueColor: shadeOrTint(desaturate(adjust-hue($euiColorPrimary, 22), 22.95), 26%, 100%) !default;

$euiFormBorderColor: transparentize($euiFormBorderOpaqueColor, .9) !default;

As you can see, $euiFormBorderOpaqueColor is unnecessarily wild (desaturating our primary color?? why?) which is likely why we moved away from it and to euiTheme.border.color (much simpler).

However, transparentize 0.9 is not the right amount compared to production, which has an alpha of 0.1 compared to 0.9 - we needed to flip that to 0.1 to match production, otherwise the box-shadow overlaps the bottom linear gradient underline, like so:

0.9 0.1

Again, this likely went unnoticed for a while as euiFormVariables() was not being widely used by production form components.

[adding to the thread]

@cee-chen Got it. I think I misunderstood some of your commit messages. I'm good with these changes.

You're totally fine, honestly this was kind of a lot to deal with conversion-wise, and I got pretty lost in the sauce of trying to figure out what was working and what wasn't + fixing existing mistakes in previous conversions. Thanks for making me slow down and write this all out in plain English!

- transparency is way not low enough right now, it needs to be `0.1` in order to not sit on top of the underline background linear gradient - as a result we need to darken the heck out of the border color - still not a 1:1 migration as the old color's Sass calculations were semi-ludicrous, but definitely close enough + replace static `1px` with a token fk

- not sure why we weren't setting it before! readonly styles will also reuse this + simplify `euiPlaceholderPerBrowser` to be more agnostic

- goal is to reuse these DRYly where possible

- we should use `euiFormControlStyles` instead + fix one EuiRange usage to just use `euiFormVariables` directly instead, the extra logic is really pretty unnecessary

- use inline styles to set the CSS variables dynamically based on props, which lets us reduce compressed overrides - store icon size affordances in variable obj

- `.euiFormControlLayout--group` still needs to be converted separately, but I'll grab this as a separate PR

- static classNames no longer work, but thankfully we can just set a CSS variable now which inherits automatically, wee - clean up group styles significantly as well, lots of things no longer needed after Emotion

…its classNames - icon affordances are still somewhat repeated, but will have to wait for a future PR

…pressed inputs + simplify CSS to use `height: 100%` instead + clean up conditional Emotion styles

- fixes/removes `--inGroup` modifier className usage in docs - not sure if this is the best approach long-term, but it's fairly straightforward for now. I'll need to re-evaluate at the end of the full migration

- more specific in name / it's clearer that it referes to 1 element of the form rather than the entire form - better matches the other CSS var we just added

- replace with actual component

- not sure what that `!important` was doing there 🤦

- Borked this when I moved `border: none` to `euiFormControlDefaultShadow`

cee-chen · 2024-05-20T20:03:14Z

Thanks for helping me slow down and catch several issues Jason! I'll go ahead and merge (a little more YOLO-y than I would normally, as this is going into a feature branch) once staging is done building.

kibanamachine · 2024-05-20T20:08:29Z

Preview staging links for this PR:

Docs site: https://eui.elastic.co/pr_7770/
Storybook: https://eui.elastic.co/pr_7770/storybook

elasticmachine · 2024-05-20T20:08:33Z

💚 Build Succeeded

Buildkite Build
Commit: 5f59c90

History

💚 Build #2000 succeeded 9c1f7b8
💚 Build #1998 succeeded 98c687d
💚 Build #1993 succeeded 6edee4c
💚 Build #1992 succeeded 5a186a5
💔 Build #1991 failed f3efeb2

Fix disabled background color

f7b6719

- this wasn't migrated correctly 20 months ago, `0.05` = `2%`

cee-chen force-pushed the emotion/field_text branch from f3efeb2 to 5a186a5 Compare May 17, 2024 22:29

cee-chen marked this pull request as ready for review May 17, 2024 23:50

cee-chen requested a review from a team as a code owner May 17, 2024 23:50

JasonStoltz self-requested a review May 20, 2024 16:25

JasonStoltz reviewed May 20, 2024

View reviewed changes

packages/eui/src/components/form/form.styles.ts Show resolved Hide resolved

packages/eui/src/components/date_picker/date_picker_range.styles.ts Show resolved Hide resolved

packages/eui/changelogs/upcoming/7770.md Show resolved Hide resolved

cee-chen force-pushed the emotion/field_text branch from 6edee4c to 973f499 Compare May 20, 2024 17:43

cee-chen force-pushed the emotion/field_text branch 2 times, most recently from ff8a5a3 to 9c1f7b8 Compare May 20, 2024 18:53

cee-chen commented May 20, 2024

View reviewed changes

packages/eui/src/components/form/form.styles.ts Show resolved Hide resolved

JasonStoltz approved these changes May 20, 2024

View reviewed changes

cee-chen commented May 20, 2024

View reviewed changes

cee-chen added 15 commits May 20, 2024 12:38

[cleanup] Pull out DRY form.textColor variable

1d0fdd6

- not sure why we weren't setting it before! readonly styles will also reuse this + simplify `euiPlaceholderPerBrowser` to be more agnostic

Convert various Sass modifier mixins to Emotion

bc652c8

- goal is to reuse these DRYly where possible

🔥 Remove old euiFormControlSize util

42fe6cd

- we should use `euiFormControlStyles` instead + fix one EuiRange usage to just use `euiFormVariables` directly instead, the extra logic is really pretty unnecessary

[EuiFieldText] Convert to new Emotion styles

5501d49

Refactor icons padding affordance with CSS variables and calc()

4ddc0ef

- use inline styles to set the CSS variables dynamically based on props, which lets us reduce compressed overrides - store icon size affordances in variable obj

Convert remaining inGroup modifier class

eba29be

- `.euiFormControlLayout--group` still needs to be converted separately, but I'll grab this as a separate PR

Remove EuiFieldText CSS files

4a1d1d3

[EuiColorPicker] Fix/update EuiFieldText usage

03dc300

- static classNames no longer work, but thankfully we can just set a CSS variable now which inherits automatically, wee - clean up group styles significantly as well, lots of things no longer needed after Emotion

[EuiDatePicker] Update to dogfood EuiFieldText directly instead of …

e13aa41

…its classNames - icon affordances are still somewhat repeated, but will have to wait for a future PR

[EuiDatePickerRange] Fix broken invalid/focus linear gradient for com…

e96fec1

…pressed inputs + simplify CSS to use `height: 100%` instead + clean up conditional Emotion styles

Update remaining downstream EuiFieldText snapshots

2ceb2d8

[EuiFieldText] Simplify controlOnly inGroup CSS

0c12b03

- fixes/removes `--inGroup` modifier className usage in docs - not sure if this is the best approach long-term, but it's fairly straightforward for now. I'll need to re-evaluate at the end of the full migration

[misc] Rename --euiFormStateColor to --euiFormControlStateColor

090100e

- more specific in name / it's clearer that it referes to 1 element of the form rather than the entire form - better matches the other CSS var we just added

changelog

bb75c1d

cee-chen added 3 commits May 20, 2024 12:38

[docs] Fix inline euiFieldText className usage

dada663

- replace with actual component

[PR feedback] Fix :focus:invalid styles

39c180e

- not sure what that `!important` was doing there 🤦

Fix EuiMarkdownEditor missing border

5f59c90

- Borked this when I moved `border: none` to `euiFormControlDefaultShadow`

cee-chen force-pushed the emotion/field_text branch from 9c1f7b8 to 5f59c90 Compare May 20, 2024 19:40

cee-chen merged commit 71c016a into elastic:emotion/forms May 20, 2024
4 checks passed

cee-chen deleted the emotion/field_text branch May 20, 2024 20:56

This was referenced May 29, 2024

[EuiFormControlLayout] Automatically handle icon affordance via CSS variables #7799

Merged

[Emotion] Convert EuiFieldNumber, EuiFieldSearch, and EuiFieldPassword #7802

Merged

cee-chen added the emotion label Jun 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Emotion] Convert `EuiFieldText` #7770

[Emotion] Convert `EuiFieldText` #7770

cee-chen commented May 17, 2024 •

edited

JasonStoltz commented May 20, 2024

cee-chen commented May 20, 2024

JasonStoltz left a comment

cee-chen commented May 20, 2024 •

edited

JasonStoltz left a comment

cee-chen May 20, 2024 •

edited

cee-chen May 20, 2024 •

edited

cee-chen commented May 20, 2024

kibanamachine commented May 20, 2024

elasticmachine commented May 20, 2024

	$euiFormBorderOpaqueColor: shadeOrTint(desaturate(adjust-hue($euiColorPrimary, 22), 22.95), 26%, 100%) !default;
	$euiFormBorderColor: transparentize($euiFormBorderOpaqueColor, .9) !default;

[Emotion] Convert EuiFieldText #7770

[Emotion] Convert EuiFieldText #7770

Conversation

cee-chen commented May 17, 2024 • edited

Summary

QA

General checklist

JasonStoltz commented May 20, 2024

cee-chen commented May 20, 2024

JasonStoltz left a comment

Choose a reason for hiding this comment

cee-chen commented May 20, 2024 • edited

JasonStoltz left a comment

Choose a reason for hiding this comment

cee-chen May 20, 2024 • edited

Choose a reason for hiding this comment

cee-chen May 20, 2024 • edited

Choose a reason for hiding this comment

cee-chen commented May 20, 2024

kibanamachine commented May 20, 2024

elasticmachine commented May 20, 2024

💚 Build Succeeded

History

[Emotion] Convert `EuiFieldText` #7770

[Emotion] Convert `EuiFieldText` #7770

cee-chen commented May 17, 2024 •

edited

cee-chen commented May 20, 2024 •

edited

cee-chen May 20, 2024 •

edited

cee-chen May 20, 2024 •

edited