[Emotion] Convert `euiFocusRing()` mixin #5855

cchaos · 2022-04-29T20:02:01Z

My general thought process here is that we want to limit the variances in focus states across components, but we know that there are some places where we at least need to adjust the position (inset/outset) of the outline or better control the color. This is mainly a component driven and therefore, will can be adjusted through the new function.

At the global level, however, I can see a consumer (and eventually user) wanting to adjust some of the settings more broadly like making the outline thicker or making them all a specific color. These are the only tokens that can be adjusted.

Since we "pride" ourselves in accessibility, this pattern enforces consumers to use the standard outline property for focus rings and standardizes them across all components for consistency.

Global token

I've re-instated the top-level focus token on EuiTheme but reduced the original type to just color and width. I've left in the comments from the other keys for now as I'll revisit background colors and transparency later.

Mixin -> Function

The new euiFocusRing()/useEuiFocusRing() function/hook returns a full css`` block of styles necessary to customize focus outlines with the ability to adjust color and `offset`.

Basic usage:

css`
  &:focus {
    ${useEuiFocusRing('outset', euiTheme.colors.primary)}
  }
`

You can also apply it un-scoped to :focus since outlines only apply to focus state anyway.

Global reset

The reset file has been updated to use the function directly so that we're only creating these styles once. It also fixed the current offset in the reset which was not get calculated correctly. The following screenshot compares the style output based on the 3 usages (none, without :focus, with :focus):

Checklist

Checked in both light and dark modes
[ ] Checked in mobile
Checked in Chrome, Safari, Edge, and Firefox
Props have proper autodocs and playground toggles
Added documentation
~~[ ] Checked Code Sandbox works for any docs examples~~
Added or updated jest and cypress tests
~~[ ] Checked for breaking changes and labeled appropriately~~
Checked for accessibility including keyboard-only and screenreader modes
~~[ ] Updated the Figma library counterpart~~
A changelog entry exists and is marked appropriately

… section

Updated ThemeExamples code snippets to scroll horizontally

cchaos · 2022-04-29T20:08:56Z

src-docs/src/views/theme/_components/_theme_example.tsx

                {finalSnippet && (
                  <EuiCodeBlock
+                    whiteSpace="pre"


I'm now forcing these snippets to scroll horizontally because the wrapping causes weird line-breaks making it hard to read.

cchaos · 2022-04-29T20:11:11Z

src-docs/src/views/theme/customizing/_values.tsx

@@ -58,6 +59,7 @@ export const ThemeValue: FunctionComponent<ThemeValue> = ({
  numberProps,
  stringProps,
  colorProps,
+  forceUpdateType,


The theme value customizing component now accepts a forced type for rendering the update form component. For instance, the type for focus.color is your typical ColorModeSwitch but we're just setting it to a simple string currentColor so I'm forcing the updating form element to render a text input instead of the color picker.

cchaos · 2022-04-29T20:12:25Z

src/components/panel/split_panel/_split_panel.scss

+    &.euiSplitPanel-isResponsive.euiPanel--#{$modifier} > .euiSplitPanel__inner,
+    &.euiPanel--#{$modifier} > .euiSplitPanel__inner {


Aside fix:

I needed to fix the scope of the border radius' for nested split panels.

cchaos · 2022-04-29T20:13:24Z

src/global_styling/reset/global_styles.tsx

-    // 👉 Safari and Firefox innately respect only showing the outline with keyboard only
-    // 💔 But they don't allow coloring of the 'auto'/default outline, so contrast is no good in dark mode.
-    // 👉 For these browsers we use the solid type in order to match with \`currentColor\`.
-    // 😦 Which does means the outline will be square


All these comments moved to the function.

cchaos · 2022-04-29T20:14:16Z

src/global_styling/variables/states.ts

@@ -11,7 +11,7 @@ import { CSSProperties } from 'react';

 /**
 * NOTE: These were quick conversions of their Sass counterparts.
- *       They have yet to be used/tested.
+ *       The commented out keys have not been established as necessary yet.


Keeping unused but already "converted" values in these files til I can get to them.

cchaos · 2022-04-29T20:33:39Z

src/global_styling/mixins/_states.ts

+ * This re-applies the same default outline with a couple parameters
+ * @param euiTheme UseEuiTheme.euiTheme
+ * @param offset Accepts a specific measurement or 'inset' or 'outset' or 'center' to adjust outline position
+ * @param color Accepts any CSS color, **Note: only works in -webkit-**


Note to reviewers too: Firefox does not support the color parameter on outline.

kibanamachine · 2022-04-29T21:02:30Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_5855/

elizabetdev

Thanks, @cchaos. Tested locally in Chrome, Firefox, and Safari. LGTM! 🎉

src/global_styling/variables/states.ts

elizabetdev · 2022-05-02T18:44:01Z

src-docs/src/views/accessibility/styles_helpers.tsx

+          <p>
+            By default, all interactable elements will inherit the{' '}
+            <EuiCode>outline</EuiCode> property from the reset file. However,
+            some instances require adjustment to the <EuiCode>offset</EuiCode>{' '}
+            and <EuiCode>color</EuiCode> of this outline. This helper function
+            allows that customization of the focus outline.
+          </p>


Maybe you should list what are the available offset options: 'inset' | 'outset' | 'center' | CSSProperties['outlineOffset']. I'm just thinking what if I'm using this hook without TS?

Yeah it starts getting into the territory of having to duplicate prop types. I wonder if there's a way we can do this programmatically instead.

I went ahead and added this one in manually, making space for it in the re-used component ThemeExample. I'm hoping someone smarter than me will be able to create a re-usable service 😸

Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co>

kibanamachine · 2022-05-02T19:48:49Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_5855/

kibanamachine · 2022-05-02T20:20:02Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_5855/

kibanamachine · 2022-05-02T22:26:05Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_5855/

thompsongl

Code changes LGTM!

* Commented out some unused (yet) properties and updated the Focus docs section * Make `focus` global token required * Updated ThemeExamples code snippets to scroll horizontally * [EuiSplitPanel] Fix border-radius of nested split panels # Conflicts: # src/global_styling/mixins/index.ts

cchaos added 6 commits April 29, 2022 00:34

WIP: Initial setup of euiFocustRing mixin

cbf13cb

Added color configuration for focus ring and docs

c8fe913

Commented out some unused (yet) properties and updated the Focus docs…

c7da030

… section

Make focus global token required

20e41ae

[EuiSplitPanel] Fix border-radius of nested split panels

5b83b06

Updated ThemeExamples code snippets to scroll horizontally

Return static string not literal, added tests

4ccba04

cchaos commented Apr 29, 2022

View reviewed changes

Added a note about -webkit- only for color

35d18cc

cchaos commented Apr 29, 2022

View reviewed changes

elizabetdev approved these changes May 2, 2022

View reviewed changes

Update src/global_styling/variables/states.ts

7b806b9

Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co>

Manually adding prop types

10e85d9

cchaos requested a review from thompsongl May 2, 2022 19:52

Cleanup code comments

9eb5de0

cchaos mentioned this pull request May 2, 2022

[Emotion] Logical property, padding, and background color utilities #5850

Merged

5 tasks

thompsongl approved these changes May 3, 2022

View reviewed changes

cchaos merged commit 89c0d55 into elastic:main May 3, 2022

cchaos deleted the css-in-js/focus branch May 3, 2022 15:43

elizabetdev mentioned this pull request May 4, 2022

[Emotion] Convert EuiLink #5856

Merged

3 tasks

breehall mentioned this pull request May 16, 2022

Upgrade EUI to v58.0.0 elastic/kibana#132257

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Emotion] Convert `euiFocusRing()` mixin #5855

[Emotion] Convert `euiFocusRing()` mixin #5855

cchaos commented Apr 29, 2022 •

edited

Loading

cchaos Apr 29, 2022

cchaos Apr 29, 2022

cchaos Apr 29, 2022

cchaos Apr 29, 2022

cchaos Apr 29, 2022

cchaos Apr 29, 2022

kibanamachine commented Apr 29, 2022

elizabetdev left a comment

elizabetdev May 2, 2022

cchaos May 2, 2022

cchaos May 2, 2022

kibanamachine commented May 2, 2022

kibanamachine commented May 2, 2022

kibanamachine commented May 2, 2022

thompsongl left a comment

		&.euiSplitPanel-isResponsive.euiPanel--#{$modifier} > .euiSplitPanel__inner,
		&.euiPanel--#{$modifier} > .euiSplitPanel__inner {

[Emotion] Convert euiFocusRing() mixin #5855

[Emotion] Convert euiFocusRing() mixin #5855

Conversation

cchaos commented Apr 29, 2022 • edited Loading

Global token

Mixin -> Function

Global reset

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aside fix:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Apr 29, 2022

elizabetdev 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

kibanamachine commented May 2, 2022

kibanamachine commented May 2, 2022

kibanamachine commented May 2, 2022

thompsongl left a comment

Choose a reason for hiding this comment

[Emotion] Convert `euiFocusRing()` mixin #5855

[Emotion] Convert `euiFocusRing()` mixin #5855

cchaos commented Apr 29, 2022 •

edited

Loading