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

cchaos · 2022-04-28T04:39:26Z

These added utilities are some of the most commonly used/re-used properties among EUI and consumer styles.

Logical properties

These help convert standard direction properties like left, height, and margin-bottom into logical properties like inline-start, inset-block, and margin-block-end.

Docs lives under Theming/SIze: https://eui.elastic.co/pr_5850/#/theming/sizing#utilities

**NOTE: ** I did not prefix these helpers with eui because they're generic and not specific to EUI in any way.

logicals{}: Map of all the directional properties to the logical counterpart
~~logicalCSS(): Function to return the string version of the property: value pair~~
logicalStyle(): Function to return the object version of the property: value pair
logicalTextAlign(): Text-specific function because it's the value that needs to change not the property

Padding

These helpers either create the full map of common padding sizes (styles function) including logical side or quickly grab one of the pre-defined padding attributes based on side and size (hook).

Docs lives under Theming/Size: https://eui.elastic.co/pr_5850/#/theming/sizing#utilities

euiPaddingSize(): Returns the calculated padding size value only
useEuiPaddingSize(): Hook version of ^^
useEuiPaddingStyles(): Returns the full map of size to padding value for quick styling of components that can have their padding size changed via a prop. It also accepts a side that will update the property based on the logical property equivelant.

Background color

Similar to the padding functions, these create maps specifically around colorizing background colors based off the brand colors (lightly shaded versions). It's similar to how EuiPanel colors it's background but now in a re-usable style fashion.

Docs lives under Theming/Color: https://eui.elastic.co/pr_5850/#/theming/colors#utilities

euiBackgroundColor(): just calculates and returns the color
useEuiBackgroundColor(): Hook that returns the string version calculated color
useEuiBackgroundColorStyles(): Returns the full map of token to background-color: value pair for quick styling of components that can have their background color changed via a prop

Contrast specific docs added to the bottom of the contrast tab.

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

…xisting styles

kibanamachine · 2022-04-28T05:08:50Z

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

src-docs/src/views/theme/sizing/_sizing_js.tsx

cchaos · 2022-04-28T14:05:05Z

src/components/horizontal_rule/horizontal_rule.styles.ts

  quarter: css`
-    width: 25%;
-    margin-inline: auto;
+    ${logicalCSS('width', '25%')}
+    ${logicalCSS('margin-horizontal', 'auto')}
  `,


I've started updating some properties, but at the end of the day, if we know/remember the logical property as we see below with margin-block it's somewhat easier to read. So I 🤷‍♀️ about whether we need to use these helpers everywhere or not. The two arguments I see for using the util are:

Future proof readability for folks not fully knowledgable of the new properties

If the CSS spec ever changes, we have one place we need to update (least likely scenario at this point)

cchaos · 2022-04-28T14:06:37Z

src/global_styling/mixins/_color.ts

+
+export type EuiBackgroundColor = typeof BACKGROUND_COLORS[number];
+
+export const euiBackgroundColorStyles = ({


This is basically the generic equivalent/conversion of $euiPanelBackgroundColorModifiers

cchaos · 2022-04-28T14:06:48Z

src/global_styling/mixins/_padding.ts

+export const PADDING_SIZES = ['none', 'xs', 's', 'm', 'l', 'xl'] as const;
+export type EuiPaddingSize = typeof PADDING_SIZES[number];
+
+export const euiPaddingStyles = (


This is basically the generic equivalent/conversion of $euiPanelPaddingModifiers

cchaos · 2022-04-28T14:07:35Z

Ready for review 😄

kibanamachine · 2022-04-28T14:36:54Z

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

cchaos · 2022-04-28T19:43:50Z

I took another page out of @constancecchen's book (i.e. copied her work) and created tests for these utils. This is a great method also to quickly compare the values of every acceptable parameter (and makes it easy for me to update Figma 😉 )

_might as well :)_

kibanamachine · 2022-04-28T20:21:49Z

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

kibanamachine · 2022-04-29T05:12:20Z

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

This reverts commit 972d57e.

- `euiBackgroundColorStyles` is now a hook version: `useEuiBackgroundColorStyles` - Added table of values in docs section - Added specific contrast section

- `euiPaddingSize` just passes back the value - `useEuiPadding` is the hook version - `useEuiPaddingStyles` is the hook that passes back the entire object - Fixed the value for size `m` to be `base` not `m` - Fixed up tests and docs

kibanamachine · 2022-05-02T06:25:31Z

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

kibanamachine · 2022-05-02T20:43:55Z

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

kibanamachine · 2022-05-02T22:04:10Z

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

cchaos · 2022-05-02T23:49:00Z

I think I've addressed all concerns. Thank you both! 💟

P.S. I'll update the docs' sections with parameter types when #5855 merges to utilize the new ThemeValue prop added there.

kibanamachine · 2022-05-03T00:21:06Z

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

cee-chen · 2022-05-03T01:01:27Z

Oh no! I'm not sure what's going wrong, but it looks like Emotion isn't rendering the expected output on https://eui.elastic.co/pr_5850/#/display/avatar 😬

Maybe it's the semi-colons following the mixin usage? I'm not actually sure they should be included when returning an object

EDIT: Yeah I think it's the semi-colons. https://eui.elastic.co/pr_5850/#/layout/horizontal-rule works and it has no trailing semi-colons.

cchaos · 2022-05-03T02:22:05Z

Womp womp. It's not just the semicolons. It seems passing back an object expects it to be very precisely applied. After removing them, I still got the following css output:

    overflow-block: hidden;
    font-weight: 500;
    [object Object] [object Object] line-height: calc(16px * 4);

It's because of the extra wrapping function that was supposed to just return a string:

eui/src/components/avatar/avatar.styles.ts

Lines 20 to 22 in ded07ab

    
             return ` 
        
               ${logicalStyle('width', size)}; 
        
               ${logicalStyle('height', size)};

I can change this particular instance to return a serialized string, but I think we may run into this more often than not since we're trying to always return strings.

That's why I think optimizing for the string value is better than optimizing for the object. Still begs the question though of whether we need to have two mixins per.

cee-chen · 2022-05-03T06:27:53Z

D'oh. Totally makes sense. I'd be good with consistently returning strings personally. I do think we should prefer to pick 1 or the other for developer experience. @thompsongl any thoughts here?

cee-chen · 2022-05-03T17:14:49Z

Per our sync today, I'd be good with reverting ded07ab and shipping this PR with both string and object utils!

This reverts commit 29c6642.

This reverts commit ded07ab.

Restrict the GitHub token permissions only to the required ones; this way, even if the attackers will succeed in compromising your workflow, they won’t be able to do much. - Included permissions for the action. https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#permissions https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs [Keeping your GitHub Actions and workflows secure Part 1: Preventing pwn requests](https://securitylab.github.com/research/github-actions-preventing-pwn-requests/) Signed-off-by: naveen <[email protected]>

* 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

…orCSS`

# Conflicts: # src/global_styling/mixins/index.ts

cchaos · 2022-05-03T18:45:12Z

Sorry about the commit history, but this PR should be up to date now based on the discussion. 😅

cee-chen

🎉

kibanamachine · 2022-05-03T19:00:54Z

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

kibanamachine · 2022-05-03T19:13:20Z

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

elizabetdev

LGTM! 🎉

cchaos added 8 commits April 27, 2022 16:32

WIP Logicals

1f1de84

Added padding utils

83e0a1f

Added background color utils

977d845

Increasing the logical properties

e70d3f0

Moved logical utils to Theming/Size

bc56cbf

Moved padding utils to Theming/Size

0804078

Moved background color utils to Theming/Color

9894542

Added separate text-align logical props and started updating some e…

de889dd

…xisting styles

cl

e321d46

cchaos commented Apr 28, 2022

View reviewed changes

cchaos marked this pull request as ready for review April 28, 2022 14:07

elizabetdev self-requested a review April 28, 2022 17:34

cchaos added 4 commits April 28, 2022 15:15

Merge remote-tracking branch 'origin/main' into css-in-js/utils

3ef3975

Added tests for padding hook

c170cd1

Added tests for logical functions

8c7cd2c

Added tests for background-color util

ac5c03d

Added test for typography

972d57e

_might as well :)_

Fix order of font size measurements to have rem first

cc21b63

cchaos added 5 commits May 1, 2022 23:45

Revert "Added test for typography"

468ac79

This reverts commit 972d57e.

Added simplified euiBackgroundColor for calculating just the color

87b964f

- `euiBackgroundColorStyles` is now a hook version: `useEuiBackgroundColorStyles` - Added table of values in docs section - Added specific contrast section

Merge remote-tracking branch 'origin/main' into css-in-js/utils

6b9fa3b

Updated padding in the same way

652ae22

- `euiPaddingSize` just passes back the value - `useEuiPadding` is the hook version - `useEuiPaddingStyles` is the hook that passes back the entire object - Fixed the value for size `m` to be `base` not `m` - Fixed up tests and docs

Naming fix & CL

c30eea4

cchaos added 2 commits May 2, 2022 16:12

More types/tests for text-align

3ec5af9

Uniform styles

2f4ded4

camelCase logicStyle object return

4a3a034

cchaos added 2 commits May 2, 2022 19:44

Remove logicalCSS() in favor of logicalStyle()

ded07ab

Update cl

29c6642

cchaos requested a review from cee-chen May 2, 2022 23:49

cchaos and others added 8 commits May 3, 2022 13:23

Revert "Update cl"

162be46

This reverts commit 29c6642.

Revert "Remove logicalCSS() in favor of logicalStyle()"

c2d64ec

This reverts commit ded07ab.

Add CSS and Style version of text-align logical mixin

921c6ae

Renamed useEuiBackgroundColorStyles to useEuiBackgroundColorCSS

857ca4e

Changed name of useEuiBackgroundColorStyles to `useEuiBackgroundCol…

89c551a

…orCSS`

Merge remote-tracking branch 'origin/main' into css-in-js/utils

5f08efa

# Conflicts: # src/global_styling/mixins/index.ts

cee-chen approved these changes May 3, 2022

View reviewed changes

elizabetdev approved these changes May 4, 2022

View reviewed changes

cchaos merged commit 1c780b1 into elastic:main May 4, 2022

cchaos deleted the css-in-js/utils branch May 4, 2022 15:27

breehall mentioned this pull request May 16, 2022

Upgrade EUI to v58.0.0 elastic/kibana#132257

Merged


		export type EuiBackgroundColor = typeof BACKGROUND_COLORS[number];

		export const euiBackgroundColorStyles = ({

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

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

Uh oh!

Conversation

cchaos commented Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

These added utilities are some of the most commonly used/re-used properties among EUI and consumer styles.

Logical properties

Padding

Background color

Checklist

Uh oh!

kibanamachine commented Apr 28, 2022

Uh oh!

Uh oh!

cchaos Apr 28, 2022

Choose a reason for hiding this comment

Uh oh!

cchaos Apr 28, 2022

Choose a reason for hiding this comment

Uh oh!

cchaos Apr 28, 2022

Choose a reason for hiding this comment

Uh oh!

cchaos commented Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kibanamachine commented Apr 28, 2022

Uh oh!

cchaos commented Apr 28, 2022

Uh oh!

kibanamachine commented Apr 28, 2022

Uh oh!

kibanamachine commented Apr 29, 2022

Uh oh!

kibanamachine commented May 2, 2022

Uh oh!

kibanamachine commented May 2, 2022

Uh oh!

kibanamachine commented May 2, 2022

Uh oh!

cchaos commented May 2, 2022

Uh oh!

kibanamachine commented May 3, 2022

Uh oh!

cee-chen commented May 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cchaos commented May 3, 2022

Uh oh!

cee-chen commented May 3, 2022

Uh oh!

cee-chen commented May 3, 2022

Uh oh!

cchaos commented May 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cee-chen left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented May 3, 2022

Uh oh!

kibanamachine commented May 3, 2022

Uh oh!

elizabetdev left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cchaos commented Apr 28, 2022 •

edited

Loading

cchaos commented Apr 28, 2022 •

edited

Loading

cee-chen commented May 3, 2022 •

edited

Loading

cchaos commented May 3, 2022 •

edited

Loading