Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Emotion] Convert global className utilities #6124

Merged
merged 10 commits into from
Aug 16, 2022
Merged

[Emotion] Convert global className utilities #6124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c88f1ea
Convert text globals/utilities to Emotion
cee-chen Aug 10, 2022
28d626b
Pass euiTheme to globalStyles and convert euiNumberFormat
cee-chen Aug 10, 2022
b1d25fa
Convert display utils to Emotion
cee-chen Aug 10, 2022
5c2d5ef
Document euiFullHeight under scroll utils
cee-chen Aug 10, 2022
48fe87f
Convert scroll globals to Emotion (+ add logical CSS properties)
cee-chen Aug 10, 2022
a557e69
Convert .eui-isFocusable global to Emotion
cee-chen Aug 10, 2022
3f4187e
Convert responsive globals to Emotion
cee-chen Aug 10, 2022
ab32f6f
Add snapshot test for quick checking of global styles output
cee-chen Aug 11, 2022
3e288c0
Changelog
cee-chen Aug 11, 2022
626c66f
Remove `.eui-isFocusable` - not being used anywhere
cee-chen Aug 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 65 additions & 21 deletions .../views/scroll/utility_classes_overflow.js → src-docs/src/views/scroll/full_height.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { useContext } from 'react';

import {
EuiText,
Expand All @@ -7,13 +7,18 @@ import {
EuiFlexItem,
EuiFlexGroup,
} from '../../../../src';
import { UtilityClassesSection } from '../utility_classes/utility_classes_section';
import { ThemeContext } from '../../components/with_theme';
import { ThemeExample } from '../theme/_components/_theme_example';

export default () => {
const themeContext = useContext(ThemeContext);
const currentLanguage = themeContext.themeLanguage;
const showSass = currentLanguage.includes('sass');

return (
<>
<UtilityClassesSection
code=".eui-fullHeight"
<ThemeExample
title=".eui-fullHeight"
type="className"
description={
<>
Expand All @@ -22,19 +27,7 @@ export default () => {
parents dimensions. Use it to stretch each nested element until
the one that applies scroll.
</p>
<p>
It applies{' '}
<EuiCode language="sass">height: 100%; overflow: hidden;</EuiCode>{' '}
but also adds <EuiCode language="sass">flex: 1 1 auto;</EuiCode>{' '}
for uses within <EuiCode language="sass">flex</EuiCode>{' '}
containers.
</p>
<dl>
<dt>Sass mixins</dt>
<dd>
<EuiCode language="scss">@include euiFullHeight;</EuiCode>
</dd>
</dl>
<p>Works on both flex and non-flex elements.</p>
</>
}
example={
Expand All @@ -48,7 +41,7 @@ export default () => {
<EuiPanel
className="eui-yScroll"
color="warning"
tabIndex="0"
tabIndex={0}
role="region"
aria-label="Example 1 for full height region"
>
Expand All @@ -67,7 +60,7 @@ export default () => {
<EuiPanel
className="eui-yScroll"
color="warning"
tabIndex="0"
tabIndex={0}
role="region"
aria-label="Example 2 for full height region"
>
Expand All @@ -90,15 +83,66 @@ export default () => {
className="eui-fullHeight" responsive={false}>
<EuiFlexItem>
<BodyScroll
className="eui-yScroll" tabIndex="0" role="region" aria-label=""/>
className="eui-yScroll" tabIndex={0} role="region" aria-label=""/>
</EuiFlexItem>
<EuiFlexItem>
<BodyScroll
className="eui-yScroll" tabIndex="0" role="region" aria-label=""/>
className="eui-yScroll" tabIndex={0} role="region" aria-label=""/>
</EuiFlexItem>
</EuiFlexGroup>
</BodyContent>`}
/>

{!showSass && (
<ThemeExample
title={<code>{'euiFullHeight()'}</code>}
type="function"
description={
<>
<p>
Emotion mixin for adding full height scrolling to a container or
flex child.
</p>
<p>
It applies{' '}
<EuiCode language="css">
height: 100%; overflow: hidden;
</EuiCode>{' '}
but also adds <EuiCode language="css">flex: 1 1 auto;</EuiCode>{' '}
for use within <EuiCode>flex</EuiCode> containers.
</p>
</>
}
snippet="${euiFullHeight()}"
snippetLanguage="emotion"
/>
)}

{showSass && (
<ThemeExample
title={<code>{'@include euiFullHeight'}</code>}
type="mixin"
description={
<>
<p>
Sass mixin for adding full height scrolling to a container or
flex child.
</p>
<p>
It applies{' '}
<EuiCode language="sass">
height: 100%; overflow: hidden;
</EuiCode>{' '}
but also adds <EuiCode language="sass">flex: 1 1 auto;</EuiCode>{' '}
for uses within <EuiCode language="sass">flex</EuiCode>{' '}
containers.
</p>
</>
}
snippet="@include euiFullHeight;"
snippetLanguage="sass"
/>
)}
</>
);
};
8 changes: 2 additions & 6 deletions src-docs/src/views/scroll/scroll_example.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import {
} from '../../../../src';

import ScrollBar from './scroll';
import Utilities from './utility_classes_overflow';
import ScrollX from './scroll_x';
import ScrollY from './scroll_y';
import FullHeight from './full_height';

export const ScrollExample = {
title: 'Scroll',
Expand Down Expand Up @@ -101,11 +101,7 @@ export const ScrollExample = {
title: 'Full height layout',
color: 'subdued',
wrapText: false,
text: (
<>
<Utilities />
</>
),
text: <FullHeight />,
},
],
};
2 changes: 1 addition & 1 deletion src-docs/src/views/scroll/scroll_x.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export default () => {
<p>
To mask the top and bottom of the scrolled content, indicating
visually that there is more content below, pass in true to the
second paremeter <EuiCode>mask</EuiCode>.
second parameter <EuiCode>mask</EuiCode>.
</p>
<p>
<EuiCode>{"useEuiOverflowScroll('x', true);"}</EuiCode>
Copy link
Contributor

@miukimiu miukimiu Aug 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something that we need to decide as a team. We normally document the hooks instead of the function versions. But I use much more functions than hooks. Why? Because in component.styles.ts we can't use hooks.

If we search for useEuiShadow we find 0 usages in EUI. But euiShadow is being used multiple times. So I wonder if we should be documenting both functions and hooks or prefer documenting the functions rather than hooks.

Copy link
Member Author

@cee-chen cee-chen Aug 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our docs are consumer facing, so we document the hooks because we expect consumers to be using hooks more than passing our euiThemeContext around. Internally in EUI we pass our theme around a lot, but consumers are much less likely to do so, so they'll probably just want a hook that handles getting the theme for them.

TBH, this is kind of an architectural choice we made too when setting up our Emotion styles pattern. We could have decided to make all our styles files/functions hooks instead, e.g. useEuiComponentStyles() which then allows us to call hook utils of all our styles instead of having to pass euiTheme around manually. But then hooks can't be called conditionally or in callbacks, so there's pros/cons to everything.

cc @thompsongl for more thoughts

Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/views/scroll/scroll_y.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export default () => {
<p>
To mask the top and bottom of the scrolled content, indicating
visually that there is more content below, pass in true to the
second paremeter <EuiCode>mask</EuiCode>.
second parameter <EuiCode>mask</EuiCode>.
</p>
<p>
<EuiCode>{"useEuiOverflowScroll('y', true);"}</EuiCode>
Expand Down
6 changes: 3 additions & 3 deletions src/components/breadcrumbs/breadcrumb.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,10 @@ export const euiBreadcrumbContentStyles = (euiThemeContext: UseEuiTheme) => {
// Types
page: css`
&:is(a):focus {
${euiFocusRing(euiTheme, 'inset')};
${euiFocusRing(euiThemeContext, 'inset')};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much better! 🥳

}
&:is(button):focus {
${euiFocusRing(euiTheme, 'center')};
${euiFocusRing(euiThemeContext, 'center')};
}
`,
application: css`
Expand All @@ -109,7 +109,7 @@ export const euiBreadcrumbContentStyles = (euiThemeContext: UseEuiTheme) => {
color: ${euiTheme.colors.link};

:focus {
${euiFocusRing(euiTheme, 'inset')};
${euiFocusRing(euiThemeContext, 'inset')};

:focus-visible {
border-radius: ${euiTheme.border.radius.medium};
Expand Down
2 changes: 1 addition & 1 deletion src/components/image/image_button.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const euiImageButtonStyles = (euiThemeContext: UseEuiTheme) => {
}

&:focus {
${euiFocusRing(euiTheme, 'outset')}
${euiFocusRing(euiThemeContext, 'outset')}
}
`,
fullWidth: css`
Expand Down
10 changes: 6 additions & 4 deletions src/components/link/link.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ export const euiLinkFocusCSS = (euiTheme: UseEuiTheme['euiTheme']) => {
`;
};

export const euiLinkCSS = (euiTheme: UseEuiTheme['euiTheme']) => {
export const euiLinkCSS = (euiThemeContext: UseEuiTheme) => {
const { euiTheme } = euiThemeContext;
return `
font-weight: ${euiTheme.font.weight.medium};
text-align: left;
Expand All @@ -49,16 +50,17 @@ export const euiLinkCSS = (euiTheme: UseEuiTheme['euiTheme']) => {
}

&:focus {
${euiFocusRing(euiTheme, 'outset')}
${euiFocusRing(euiThemeContext, 'outset')}
${euiLinkFocusCSS(euiTheme)}
}
`;
};

export const euiLinkStyles = ({ euiTheme }: UseEuiTheme) => {
export const euiLinkStyles = (euiThemeContext: UseEuiTheme) => {
const { euiTheme } = euiThemeContext;
return {
euiLink: css`
${euiLinkCSS(euiTheme)}
${euiLinkCSS(euiThemeContext)}
user-select: text;

&[target='_blank'] {
Expand Down
2 changes: 1 addition & 1 deletion src/components/text/text.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ export const euiTextStyles = (euiThemeContext: UseEuiTheme) => {
// Style anchors that don't have a class. This prevents overwriting "buttons"
// and other stylized elements passed in.
a:not([class]) {
${euiLinkCSS(euiTheme)}
${euiLinkCSS(euiThemeContext)}
}

img {
Expand Down
4 changes: 2 additions & 2 deletions src/components/toast/global_toast_list.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
euiBreakpoint,
euiScrollBarStyles,
logicalCSS,
logicalCSSWithFallback,
logicalSizeCSS,
} from '../../global_styling';
import { UseEuiTheme } from '../../services';
Expand All @@ -34,8 +35,7 @@ export const euiGlobalToastListStyles = (euiThemeContext: UseEuiTheme) => {
${logicalCSS('bottom', 0)};
${logicalCSS('width', `${euiToastWidth + euiTheme.base * 5}px`)}; /* 2 */
${logicalCSS('max-height', '100vh')}; /* 1 */
overflow-y: auto; // Fallback for the 'overflow-inline' logical property, which is not yet supported
${logicalCSS('overflow-y', 'auto')};
${logicalCSSWithFallback('overflow-y', 'auto')};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a great idea. 🥳


// Hide the scrollbar entirely
scrollbar-width: none;
Expand Down
15 changes: 11 additions & 4 deletions src/global_styling/functions/__snapshots__/logicals.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ exports[`logicalCSS mixin returns a string property for each directional propert

exports[`logicalCSS mixin returns a string property for each directional property: min-width 1`] = `"min-inline-size: 8px;"`;

exports[`logicalCSS mixin returns a string property for each directional property: overflow-x 1`] = `"overflow-block: 8px;"`;
exports[`logicalCSS mixin returns a string property for each directional property: overflow-x 1`] = `"overflow-inline: 8px;"`;

exports[`logicalCSS mixin returns a string property for each directional property: overflow-y 1`] = `"overflow-inline: 8px;"`;
exports[`logicalCSS mixin returns a string property for each directional property: overflow-y 1`] = `"overflow-block: 8px;"`;

exports[`logicalCSS mixin returns a string property for each directional property: padding-bottom 1`] = `"padding-block-end: 8px;"`;

Expand All @@ -110,6 +110,13 @@ exports[`logicalCSS mixin returns a string property for each directional propert

exports[`logicalCSS mixin returns a string property for each directional property: width 1`] = `"inline-size: 8px;"`;

exports[`logicalCSSWithFallback returns both the original property and the logical property 1`] = `
"
overflow-x: auto;
overflow-inline: auto;
"
`;

exports[`logicalStyle mixin returns an object property for each directional property: border-bottom 1`] = `
Object {
"borderBlockEnd": "8px",
Expand Down Expand Up @@ -370,13 +377,13 @@ Object {

exports[`logicalStyle mixin returns an object property for each directional property: overflow-x 1`] = `
Object {
"overflowBlock": "8px",
"overflowInline": "8px",
}
`;

exports[`logicalStyle mixin returns an object property for each directional property: overflow-y 1`] = `
Object {
"overflowInline": "8px",
"overflowBlock": "8px",
}
`;

Expand Down
9 changes: 9 additions & 0 deletions src/global_styling/functions/logicals.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
LOGICAL_PROPERTIES,
LOGICAL_TEXT_ALIGNMENT,
logicalCSS,
logicalCSSWithFallback,
logicalStyle,
logicalTextAlignCSS,
logicalTextAlignStyle,
Expand All @@ -28,6 +29,14 @@ describe('logicalCSS mixin returns a string property', () => {
});
});

describe('logicalCSSWithFallback ', () => {
it('returns both the original property and the logical property', () => {
expect(
testCustomHook(() => logicalCSSWithFallback('overflow-x', 'auto')).return
).toMatchSnapshot();
});
});

describe('logicalStyle mixin returns an object property', () => {
describe('for each directional property:', () => {
LOGICAL_PROPERTIES.forEach((prop) => {
Expand Down
22 changes: 20 additions & 2 deletions src/global_styling/functions/logicals.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ const logicalSize = {
};

const logicalOverflow = {
'overflow-x': 'overflow-block',
'overflow-y': 'overflow-inline',
'overflow-x': 'overflow-inline',
'overflow-y': 'overflow-block',
};

const logicalBorders = {
Expand Down Expand Up @@ -122,6 +122,24 @@ export const logicalCSS = (property: LogicalProperties, value?: any) => {
return `${logicals[property]}: ${value};`;
};

/**
* Some logical properties are not yet fully supported by all browsers.
* For those cases, we should use the old property as a fallback for
* browsers missing support, while allowing supporting browsers to use
* the logical properties.
*
* Examples:
* https://caniuse.com/?search=overflow-block
* https://caniuse.com/mdn-css_properties_float_flow_relative_values
*/
export const logicalCSSWithFallback = (
property: LogicalProperties,
value?: any
) => `
${property}: ${value};
${logicalCSS(property, value)}
`;

/**
*
* @param property A string that is a valid CSS logical property
Expand Down
4 changes: 2 additions & 2 deletions src/global_styling/mixins/__snapshots__/_typography.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ exports[`euiTextBreakWord returns a string of CSS text 1`] = `

exports[`euiTextTruncate allows customizing max-width 1`] = `
"
max-width: 150px; // Ensure that the node has a maximum width after which truncation can occur
max-inline-size: 150px;
overflow: hidden !important;
text-overflow: ellipsis !important;
white-space: nowrap !important;
Expand All @@ -214,7 +214,7 @@ exports[`euiTextTruncate allows customizing max-width 1`] = `

exports[`euiTextTruncate returns a string of CSS text 1`] = `
"
max-width: 100%; // Ensure that the node has a maximum width after which truncation can occur
max-inline-size: 100%;
overflow: hidden !important;
text-overflow: ellipsis !important;
white-space: nowrap !important;
Expand Down
Loading