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

[EuiIcon, EuiButtonIcon] Improved contrast for named colors #4049

Merged
merged 13 commits into from
Sep 24, 2020
Merged

[EuiIcon, EuiButtonIcon] Improved contrast for named colors #4049

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
d778c30
Improved constrast for icons named colors
elizabetdev Sep 17, 2020
756a176
Changelog
elizabetdev Sep 17, 2020
4155a87
Improved contrast only for EuiHealth named colors
elizabetdev Sep 18, 2020
4426354
Merge remote-tracking branch 'upstream/master' into euiHealth-lighten…
elizabetdev Sep 18, 2020
82f6b4c
Merge remote-tracking branch 'upstream/master' into euiHealth-lighten…
elizabetdev Sep 21, 2020
8bd1e96
Fixing button icon color
elizabetdev Sep 21, 2020
3ec47c6
Icons colors with graphic contrast levels. Button icon colors with te…
elizabetdev Sep 23, 2020
6e4a277
Merge remote-tracking branch 'upstream/master' into euiHealth-lighten…
elizabetdev Sep 23, 2020
ff4f980
CL and snapshots
elizabetdev Sep 23, 2020
7be0458
reverting euiHealth code to use euiIcon color prop
elizabetdev Sep 24, 2020
a5bb9d6
Reverting euiHealth code
elizabetdev Sep 24, 2020
080f87f
Better CL
elizabetdev Sep 24, 2020
bc4e706
Merge remote-tracking branch 'upstream/master' into euiHealth-lighten…
elizabetdev Sep 24, 2020
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
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
## [`master`](https://github.com/elastic/eui/tree/master)

No public interface changes since `29.1.0`.
- Improved contrast for `EuiHealth`, `EuiIcon` and `EuiButtonIcon` named colors ([#4049](https://github.com/elastic/eui/pull/4049))

## [`29.1.0`](https://github.com/elastic/eui/tree/v29.1.0)

Expand Down
1 change: 0 additions & 1 deletion src-docs/src/components/guide_components.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ $guideZLevelHighest: $euiZLevel9 + 1000;
@if (lightness($euiTextColor) < 50) {
background: $euiColorDarkestShade;
}
padding: $euiSizeL;
cchaos marked this conversation as resolved.
Show resolved Hide resolved
}

.guideDemo__icon {
Expand Down
113 changes: 57 additions & 56 deletions src-docs/src/views/button/button_ghost.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
EuiFlexGroup,
EuiFlexItem,
EuiButtonToggle,
EuiPanel,
} from '../../../../src/components';

export default () => {
Expand All @@ -17,67 +18,67 @@ export default () => {
};

return (
<EuiFlexGroup
wrap
gutterSize="s"
alignItems="center"
className="guideDemo__ghostBackground">
<EuiFlexItem grow={false}>
<EuiButton color="ghost" onClick={() => window.alert('Button clicked')}>
Ghost
</EuiButton>
</EuiFlexItem>
<EuiPanel paddingSize="l" className="guideDemo__ghostBackground">
cchaos marked this conversation as resolved.
Show resolved Hide resolved
<EuiFlexGroup wrap gutterSize="s" alignItems="center">
<EuiFlexItem grow={false}>
<EuiButton
color="ghost"
onClick={() => window.alert('Button clicked')}>
Ghost
</EuiButton>
</EuiFlexItem>

<EuiFlexItem grow={false}>
<EuiButton
fill
color="ghost"
size="s"
iconType="check"
onClick={() => window.alert('Button clicked')}>
Filled
</EuiButton>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButton
fill
color="ghost"
size="s"
iconType="check"
onClick={() => window.alert('Button clicked')}>
Filled
</EuiButton>
</EuiFlexItem>

<EuiFlexItem grow={false}>
<EuiButtonEmpty
size="s"
color="ghost"
onClick={() => window.alert('Button clicked')}>
small
</EuiButtonEmpty>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButtonEmpty
size="s"
color="ghost"
onClick={() => window.alert('Button clicked')}>
small
</EuiButtonEmpty>
</EuiFlexItem>

<EuiFlexItem grow={false}>
<EuiButtonIcon
size="s"
color="ghost"
iconType="user"
onClick={() => window.alert('Button clicked')}
aria-label="Your account"
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButtonIcon
size="s"
color="ghost"
iconType="user"
onClick={() => window.alert('Button clicked')}
aria-label="Your account"
/>
</EuiFlexItem>

<EuiFlexItem grow={false}>
<EuiButton color="ghost" isLoading fill size="s">
Loading&hellip;
</EuiButton>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButton color="ghost" isLoading fill size="s">
Loading&hellip;
</EuiButton>
</EuiFlexItem>

<EuiFlexItem grow={false}>
<EuiButton color="ghost" isLoading>
Loading&hellip;
</EuiButton>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiButton color="ghost" isLoading>
Loading&hellip;
</EuiButton>
</EuiFlexItem>

<EuiFlexItem grow={false}>
<EuiButtonToggle
color="ghost"
label="Toggle Me"
fill={toggle0On}
onChange={onToggle0Change}
/>
</EuiFlexItem>
</EuiFlexGroup>
<EuiFlexItem grow={false}>
<EuiButtonToggle
color="ghost"
label="Toggle Me"
fill={toggle0On}
onChange={onToggle0Change}
/>
</EuiFlexItem>
</EuiFlexGroup>
</EuiPanel>
);
};
1 change: 1 addition & 0 deletions src-docs/src/views/button/button_icon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
const colors = [
'primary',
'text',
'accent',
'subdued',
'success',
'warning',
Expand Down
6 changes: 5 additions & 1 deletion src-docs/src/views/health/health.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ import { EuiHealth, EuiSpacer } from '../../../../src/components';

export default () => (
<div>
<EuiHealth color="subdued">Unknown</EuiHealth>
<EuiHealth color="subdued">Inactive</EuiHealth>

<EuiSpacer />

<EuiHealth color="primary">Active</EuiHealth>

<EuiSpacer />

Expand Down
13 changes: 10 additions & 3 deletions src-docs/src/views/icon/icon_colors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

import React from 'react';

import classNames from 'classnames';

import {
EuiFlexGrid,
EuiFlexItem,
Expand All @@ -24,7 +26,7 @@ import {
const iconColors = [
'default',
'primary',
'secondary',
'success',
'accent',
'warning',
'danger',
Expand All @@ -44,9 +46,14 @@ export default () => (
className="guideDemo__icon"
key={iconColor}
style={{ width: '340px' }}>
<EuiPanel>
<EuiPanel
cchaos marked this conversation as resolved.
Show resolved Hide resolved
className={classNames({
guideDemo__ghostBackground: iconColor === 'ghost',
})}>
<EuiIcon type="brush" color={iconColor} />
<EuiText size="s">
<EuiText
size="s"
color={iconColor === 'ghost' ? 'ghost' : 'default'}>
<p>{iconColor}</p>
</EuiText>
</EuiPanel>
Expand Down
8 changes: 8 additions & 0 deletions src/components/button/button_icon/__snapshots__/button_icon.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ exports[`EuiButtonIcon is rendered 1`] = `
/>
`;

exports[`EuiButtonIcon props color accent is rendered 1`] = `
<button
aria-label="button"
class="euiButtonIcon euiButtonIcon--accent"
type="button"
/>
`;

exports[`EuiButtonIcon props color danger is rendered 1`] = `
<button
aria-label="button"
Expand Down
9 changes: 5 additions & 4 deletions src/components/button/button_icon/_variables.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
// Modifier naming and colors.
$euiButtonIconTypes: (
danger: $euiColorDanger,
accent: $euiColorAccentText,
danger: $euiColorDangerText,
disabled: $euiButtonColorDisabledText,
ghost: $euiColorGhost,
primary: $euiColorPrimary,
primary: $euiColorPrimaryText,
subdued: $euiTextSubduedColor,
success: $euiColorSuccess,
success: $euiColorSuccessText,
text: $euiTextColor,
warning: $euiColorWarning,
warning: $euiColorWarningText,
);
2 changes: 2 additions & 0 deletions src/components/button/button_icon/button_icon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { IconType, IconSize, EuiIcon } from '../../icon';
import { ButtonSize } from '../button';

export type EuiButtonIconColor =
| 'accent'
elizabetdev marked this conversation as resolved.
Show resolved Hide resolved
| 'danger'
| 'disabled'
| 'ghost'
Expand Down Expand Up @@ -82,6 +83,7 @@ type Props = ExclusiveUnion<
>;

const colorToClassNameMap: { [color in EuiButtonIconColor]: string } = {
accent: 'euiButtonIcon--accent',
danger: 'euiButtonIcon--danger',
disabled: 'euiButtonIcon--disabled',
ghost: 'euiButtonIcon--ghost',
Expand Down
6 changes: 6 additions & 0 deletions src/components/health/_health.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,9 @@
@include euiFontSizeS;
display: inline-block;
}

@each $name, $color in $euiHealthColors {
.euiHealth--#{$name} {
color: $color;
}
}
3 changes: 2 additions & 1 deletion src/components/health/_index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
@import 'health';
@import './variables';
@import './health';
12 changes: 12 additions & 0 deletions src/components/health/_variables.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Modifier naming and colors.
$euiHealthColors: (
text: $euiTextColor,
subdued: $euiTextSubduedColor,
primary: makeGraphicContrastColor($euiColorPrimary),
secondary: makeGraphicContrastColor($euiColorSecondary),
success: makeGraphicContrastColor($euiColorSuccess),
accent: makeGraphicContrastColor($euiColorAccent),
warning: makeGraphicContrastColor($euiColorWarning),
danger: makeGraphicContrastColor($euiColorDanger),
ghost: $euiColorGhost,
);
44 changes: 39 additions & 5 deletions src/components/health/health.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,35 +19,69 @@

import React, { FunctionComponent, HTMLAttributes } from 'react';
import classNames from 'classnames';
import { CommonProps } from '../common';
import { CommonProps, keysOf } from '../common';

import { EuiIcon, IconColor } from '../icon';
import { EuiIcon } from '../icon';

import { EuiFlexGroup, EuiFlexItem } from '../flex';

const colorToClassMap = {
default: null,
primary: 'euiHealth--primary',
secondary: 'euiHealth--secondary',
success: 'euiHealth--success',
accent: 'euiHealth--accent',
warning: 'euiHealth--warning',
danger: 'euiHealth--danger',
text: 'euiHealth--text',
subdued: 'euiHealth--subdued',
ghost: 'euiHealth--ghost',
};

export const COLORS: NamedColor[] = keysOf(colorToClassMap);

type NamedColor = keyof typeof colorToClassMap;

// We accept arbitrary color strings, which are impossible to type.
export type HealthColor = string | NamedColor;

type EuiHealthProps = CommonProps &
Omit<HTMLAttributes<HTMLDivElement>, 'color'> & {
/**
* Sets the color of the dot icon.
* It accepts any `IconColor`: `default`, `primary`, `secondary`, `success`, `accent`, `warning`, `danger`, `text`,
* It accepts one of EUI's color palette: `default`, `primary`, `secondary`, `success`, `accent`, `warning`, `danger`, `text`,
* `subdued` or `ghost`; or any valid CSS color value as a `string`
*/
color?: IconColor;
color?: HealthColor;
};

function isNamedColor(name: string): name is NamedColor {
return colorToClassMap.hasOwnProperty(name);
}

export const EuiHealth: FunctionComponent<EuiHealthProps> = ({
children,
className,
color,
...rest
}) => {
let healthColor: any = null;

if (color) {
if (isNamedColor(color)) {
healthColor = { className: colorToClassMap[color] };
} else {
healthColor = { color: color };
}
}

const classes = classNames('euiHealth', className);

return (
<div className={classes} {...rest}>
<EuiFlexGroup gutterSize="xs" alignItems="center" responsive={false}>
<EuiFlexItem grow={false}>
<EuiIcon type="dot" color={color} />
<EuiIcon type="dot" {...healthColor} />
Copy link
Contributor

Choose a reason for hiding this comment

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

Just being nitpicky here but also want to be sure we're following our own conventions. Your classnames you've constructed that would be added here are using the BEM syntax that would be applied to the outer most element euiHealth--primary, but you're applying it to the icon which would normally be called euiHealth__icon--primary.

</EuiFlexItem>
<EuiFlexItem grow={false}>{children}</EuiFlexItem>
</EuiFlexGroup>
Expand Down
2 changes: 1 addition & 1 deletion src/components/health/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@
* under the License.
*/

export { EuiHealth } from './health';
export { EuiHealth, HealthColor, COLORS as HEALTH_COLORS } from './health';
16 changes: 8 additions & 8 deletions src/components/icon/_variables.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ $euiIconLoadingOpacity: .05;

// Modifier naming and colors.
$euiIconColors: (
text: $euiTextColor,
subdued: $euiTextSubduedColor,
primary: $euiColorPrimaryText,
secondary: $euiColorSecondaryText,
success: $euiColorSuccessText,
accent: $euiColorAccentText,
warning: $euiColorWarningText,
danger: $euiColorDangerText,
accent: makeGraphicContrastColor($euiColorAccent),
danger: makeGraphicContrastColor($euiColorDanger),
ghost: $euiColorGhost,
primary: makeGraphicContrastColor($euiColorPrimary),
secondary: makeGraphicContrastColor($euiColorSecondary),
success: makeGraphicContrastColor($euiColorSuccess),
subdued: $euiTextSubduedColor,
text: $euiTextColor,
warning: makeGraphicContrastColor($euiColorWarning),
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these all the same values used in EuiHealth? If so, I think it's ok to let the EuiIcon color handle the coloring without actually needing to re-apply the coloring in EuiHealth specifically.

Meaning passing color="accent" from EuiHealth to the embedded EuiIcon will produce the same rendered color (makeHighContrast), so why not just complicate it less.

Though there can be the argument that also having the custom coloring in EuiHealth helps ensure EuiHealth can maintain it's own coloring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reverted the EuiHealth code to what it was originally. This way the EuiIcon is handling the color for it.

This was actually how I started the PR by just changing the EuiIcon colors so I believe it's the best way.

);

$euiIconSizes: (
Expand Down