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

feat(notifications): new light/dark mode colors #1842

Merged
merged 20 commits into from
Jun 27, 2024
Merged

feat(notifications): new light/dark mode colors #1842

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
23bb0e4
feat(notifications): new light/dark mode colors
geotrev Jun 14, 2024
83325f5
feat(notifications): updates tests
geotrev Jun 14, 2024
5025b93
chore: adds colors for alert
geotrev Jun 17, 2024
07cbe24
chore: adds colors for global alert
geotrev Jun 18, 2024
b05e87a
chore: adds colors to Well
geotrev Jun 18, 2024
8991351
chore: type import in tests
geotrev Jun 18, 2024
00a21ef
chore: adds styled base container light/dark tests
geotrev Jun 19, 2024
be05ccb
chore: remove unused validationHues enum
geotrev Jun 20, 2024
1b81444
chore: adds color tests for views
geotrev Jun 20, 2024
6650cf4
chore: changes to global alert button colors (info and warning)
geotrev Jun 20, 2024
1be280c
chore: code review
geotrev Jun 21, 2024
7f119a2
chore: code review
geotrev Jun 24, 2024
54eaf91
chore: use default background in Well when not recessed
geotrev Jun 24, 2024
9244fe1
chore: adds floating background to avoid default fallback
geotrev Jun 24, 2024
ec91f22
chore: code reviewwwwww
geotrev Jun 25, 2024
6b16bab
chore: force light mode, remove dark offsets, simplify tests
geotrev Jun 25, 2024
eeedb01
chore: clean up switch in global alert view
geotrev Jun 25, 2024
0bcf703
chore: use parent theme callback
geotrev Jun 25, 2024
11836cc
chore: code review
geotrev Jun 27, 2024
9067ce0
chore: replace Object.keys
geotrev Jun 27, 2024
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
31 changes: 21 additions & 10 deletions packages/notifications/src/elements/global-alert/GlobalAlert.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* found at http://www.apache.org/licenses/LICENSE-2.0.
*/

import React, { forwardRef, useMemo } from 'react';
import React, { forwardRef, useContext, useMemo } from 'react';
import PropTypes from 'prop-types';
import InfoIcon from '@zendeskgarden/svg-icons/src/16/info-stroke.svg';
import ErrorIcon from '@zendeskgarden/svg-icons/src/16/alert-error-stroke.svg';
Expand All @@ -19,9 +19,12 @@ import { GlobalAlertButton } from './GlobalAlertButton';
import { GlobalAlertClose } from './GlobalAlertClose';
import { GlobalAlertContent } from './GlobalAlertContent';
import { GlobalAlertTitle } from './GlobalAlertTitle';
import { DefaultTheme, ThemeContext, ThemeProvider } from 'styled-components';
import { DEFAULT_THEME } from '@zendeskgarden/react-theming';

/**
* 1. role='status' on `div` is valid WAI-ARIA usage in this context.
* 1. Global Alert always renders with light theme colors
* 2. role='status' on `div` is valid WAI-ARIA usage in this context.
* https://www.w3.org/TR/wai-aria-1.1/#status
*/

Expand All @@ -33,16 +36,24 @@ const GlobalAlertComponent = forwardRef<HTMLDivElement, IGlobalAlertProps>(
warning: <WarningIcon />,
info: <InfoIcon />
}[type];
const theme = useContext(ThemeContext) || DEFAULT_THEME;
/* [1] */
const lightTheme: DefaultTheme = useMemo(
() => ({ ...theme, colors: { ...theme.colors, base: 'light' } }),
[theme]
);

return (
<GlobalAlertContext.Provider value={useMemo(() => ({ type }), [type])}>
{/* [1] */}
{/* eslint-disable-next-line jsx-a11y/prefer-tag-over-role */}
<StyledGlobalAlert ref={ref} role="status" $alertType={type} {...props}>
<StyledGlobalAlertIcon $alertType={type}>{icon}</StyledGlobalAlertIcon>
{props.children}
</StyledGlobalAlert>
</GlobalAlertContext.Provider>
<ThemeProvider theme={lightTheme}>
geotrev marked this conversation as resolved.
Show resolved Hide resolved
<GlobalAlertContext.Provider value={useMemo(() => ({ type }), [type])}>
{/* [2] */}
{/* eslint-disable-next-line jsx-a11y/prefer-tag-over-role */}
<StyledGlobalAlert ref={ref} role="status" $alertType={type} {...props}>
<StyledGlobalAlertIcon $alertType={type}>{icon}</StyledGlobalAlertIcon>
{props.children}
</StyledGlobalAlert>
</GlobalAlertContext.Provider>
</ThemeProvider>
);
}
);
Expand Down
56 changes: 22 additions & 34 deletions packages/notifications/src/styled/global-alert/StyledGlobalAlert.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,54 +6,42 @@
*/

import React from 'react';
import { getRenderFn } from 'garden-test-utils';
import { render } from 'garden-test-utils';
import { DEFAULT_THEME, PALETTE } from '@zendeskgarden/react-theming';

import { Type } from '../../types';
import { StyledGlobalAlert } from './StyledGlobalAlert';

describe('StyledGlobalAlert', () => {
it.each<{ mode: 'light' | 'dark'; type: Type; color: string }>([
{ mode: 'light', type: 'success', color: PALETTE.green[700] },
{ mode: 'dark', type: 'success', color: PALETTE.green[700] },
{ mode: 'light', type: 'error', color: PALETTE.red[700] },
{ mode: 'dark', type: 'error', color: PALETTE.red[700] },
{ mode: 'light', type: 'warning', color: PALETTE.yellow[300] },
{ mode: 'dark', type: 'warning', color: PALETTE.yellow[300] },
{ mode: 'light', type: 'info', color: PALETTE.blue[300] },
{ mode: 'dark', type: 'info', color: PALETTE.blue[300] }
])('renders $mode mode $type background color', ({ mode, type, color }) => {
const { container } = getRenderFn(mode)(<StyledGlobalAlert $alertType={type} />);
it.each<{ type: Type; color: string }>([
{ type: 'success', color: PALETTE.green[700] },
{ type: 'error', color: PALETTE.red[700] },
{ type: 'warning', color: PALETTE.yellow[300] },
{ type: 'info', color: PALETTE.blue[300] }
])('renders $mode mode $type background color', ({ type, color }) => {
const { container } = render(<StyledGlobalAlert $alertType={type} />);

expect(container.firstChild).toHaveStyleRule('background-color', color);
});

it.each<{ mode: 'light' | 'dark'; type: Type; color: string }>([
{ mode: 'light', type: 'success', color: PALETTE.green[100] },
{ mode: 'dark', type: 'success', color: PALETTE.green[100] },
{ mode: 'light', type: 'error', color: PALETTE.red[100] },
{ mode: 'dark', type: 'error', color: PALETTE.red[100] },
{ mode: 'light', type: 'warning', color: PALETTE.yellow[800] },
{ mode: 'dark', type: 'warning', color: PALETTE.yellow[800] },
{ mode: 'light', type: 'info', color: PALETTE.blue[800] },
{ mode: 'dark', type: 'info', color: PALETTE.blue[800] }
])('renders $mode mode $type foreground color', ({ mode, type, color }) => {
const { container } = getRenderFn(mode)(<StyledGlobalAlert $alertType={type} />);
it.each<{ type: Type; color: string }>([
{ type: 'success', color: PALETTE.green[100] },
{ type: 'error', color: PALETTE.red[100] },
{ type: 'warning', color: PALETTE.yellow[800] },
{ type: 'info', color: PALETTE.blue[800] }
])('renders $mode mode $type foreground color', ({ type, color }) => {
const { container } = render(<StyledGlobalAlert $alertType={type} />);

expect(container.firstChild).toHaveStyleRule('color', color);
});

it.each<{ mode: 'light' | 'dark'; type: Type; color: string }>([
{ mode: 'light', type: 'success', color: PALETTE.green[800] },
{ mode: 'dark', type: 'success', color: PALETTE.green[800] },
{ mode: 'light', type: 'error', color: PALETTE.red[800] },
{ mode: 'dark', type: 'error', color: PALETTE.red[800] },
{ mode: 'light', type: 'warning', color: PALETTE.yellow[400] },
{ mode: 'dark', type: 'warning', color: PALETTE.yellow[400] },
{ mode: 'light', type: 'info', color: PALETTE.blue[400] },
{ mode: 'dark', type: 'info', color: PALETTE.blue[400] }
])('renders $mode mode $type border color', ({ mode, type, color }) => {
const { container } = getRenderFn(mode)(<StyledGlobalAlert $alertType={type} />);
it.each<{ type: Type; color: string }>([
{ type: 'success', color: PALETTE.green[800] },
{ type: 'error', color: PALETTE.red[800] },
{ type: 'warning', color: PALETTE.yellow[400] },
{ type: 'info', color: PALETTE.blue[400] }
])('renders $mode mode $type border color', ({ type, color }) => {
const { container } = render(<StyledGlobalAlert $alertType={type} />);

expect(container.firstChild).toHaveStyleRule(
'box-shadow',
Expand Down
35 changes: 14 additions & 21 deletions packages/notifications/src/styled/global-alert/StyledGlobalAlert.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ interface IStyledGlobalAlertProps {
$alertType: IGlobalAlertProps['type'];
}

const lightDarkOptions = (lightOffset: number, darkOffset: number) => ({
light: { offset: lightOffset },
dark: { offset: darkOffset }
});

/**
* 1. Shifting :focus-visible from LVHFA order to preserve `color` on hover
*/
Expand All @@ -42,17 +37,16 @@ const colorStyles = ({ theme, $alertType }: ThemeProps<DefaultTheme> & IStyledGl
case 'success': {
borderColor = getColor({
variable: 'border.successEmphasis',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interested in thoughts around this use of emphasis vs non-emphasis. In general, I tried to use non-emphasis with foreground and emphasis with background. Does this align with design intent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note this doesn't apply across all components, and at some points foreground is using emphasis (see StyledAlert).

I'd like to ensure that when a consumer updates a color variable, it cascades through these components in the least confusing way.

Also, let me know if I'm overthinking it. 😛

...lightDarkOptions(100, 200),
light: { offset: 100 },
theme
});
backgroundColor = getColor({
variable: 'background.successEmphasis',
dark: { offset: 100 },
theme
});
foregroundColor = getColor({
variable: 'foreground.success',
...lightDarkOptions(-600, -300),
light: { offset: -600 },
theme
});
focusVariable = 'foreground.successEmphasis';
Expand All @@ -61,17 +55,16 @@ const colorStyles = ({ theme, $alertType }: ThemeProps<DefaultTheme> & IStyledGl
case 'error': {
borderColor = getColor({
variable: 'border.dangerEmphasis',
...lightDarkOptions(100, 200),
light: { offset: 100 },
theme
});
backgroundColor = getColor({
variable: 'background.dangerEmphasis',
dark: { offset: 100 },
theme
});
foregroundColor = getColor({
variable: 'foreground.danger',
...lightDarkOptions(-600, -300),
light: { offset: -600 },
theme
});
focusVariable = 'foreground.dangerEmphasis';
Expand All @@ -80,28 +73,28 @@ const colorStyles = ({ theme, $alertType }: ThemeProps<DefaultTheme> & IStyledGl
case 'warning': {
borderColor = getColor({
variable: 'border.warningEmphasis',
...lightDarkOptions(-300, -200),
light: { offset: -300 },
theme
});
backgroundColor = getColor({
variable: 'background.warningEmphasis',
...lightDarkOptions(-400, -300),
light: { offset: -400 },
theme
});
const fgVariable = 'foreground.warning';
foregroundColor = getColor({
variable: fgVariable,
...lightDarkOptions(100, 400),
light: { offset: 100 },
theme
});
anchorHoverColor = getColor({
variable: fgVariable,
...lightDarkOptions(200, 500),
light: { offset: 200 },
theme
});
anchorActiveColor = getColor({
variable: fgVariable,
...lightDarkOptions(300, 600),
light: { offset: 300 },
theme
});
focusVariable = fgVariable;
Expand All @@ -110,28 +103,28 @@ const colorStyles = ({ theme, $alertType }: ThemeProps<DefaultTheme> & IStyledGl
case 'info': {
borderColor = getColor({
variable: 'border.primaryEmphasis',
...lightDarkOptions(-300, -200),
light: { offset: -300 },
theme
});
backgroundColor = getColor({
variable: 'background.primaryEmphasis',
...lightDarkOptions(-400, -300),
light: { offset: -400 },
theme
});
const fgVariable = 'foreground.primary';
foregroundColor = getColor({
variable: fgVariable,
...lightDarkOptions(100, 200),
light: { offset: 100 },
theme
});
anchorHoverColor = getColor({
variable: fgVariable,
...lightDarkOptions(200, 300),
light: { offset: 200 },
theme
});
anchorActiveColor = getColor({
variable: fgVariable,
...lightDarkOptions(300, 400),
light: { offset: 300 },
theme
});
focusVariable = fgVariable;
Expand Down
22 changes: 8 additions & 14 deletions packages/notifications/src/styled/global-alert/StyledGlobalAlertButton.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import React from 'react';
import { getRenderFn, render } from 'garden-test-utils';
import { render } from 'garden-test-utils';
import { PALETTE } from '@zendeskgarden/react-theming';
import { StyledGlobalAlertButton } from './StyledGlobalAlertButton';
import { colorStyles } from './StyledGlobalAlertClose';
Expand All @@ -21,19 +21,13 @@ describe('StyledGlobalAlertButton', () => {
expect(colorStyles).toHaveBeenCalledTimes(1);
});

it.each<{ mode: 'light' | 'dark'; type: Type; color: string }>([
{ mode: 'light', type: 'success', color: PALETTE.green[900] },
{ mode: 'dark', type: 'success', color: PALETTE.green[900] },
{ mode: 'light', type: 'error', color: PALETTE.red[900] },
{ mode: 'dark', type: 'error', color: PALETTE.red[900] },
{ mode: 'light', type: 'warning', color: PALETTE.yellow[700] },
{ mode: 'dark', type: 'warning', color: PALETTE.yellow[700] },
{ mode: 'light', type: 'info', color: PALETTE.blue[700] },
{ mode: 'dark', type: 'info', color: PALETTE.blue[700] }
])('renders $mode mode $type background color', ({ mode, type, color }) => {
const { getByRole } = getRenderFn(mode)(
<StyledGlobalAlertButton isPrimary $alertType={type} />
);
it.each<{ type: Type; color: string }>([
{ type: 'success', color: PALETTE.green[900] },
{ type: 'error', color: PALETTE.red[900] },
{ type: 'warning', color: PALETTE.yellow[700] },
{ type: 'info', color: PALETTE.blue[700] }
])('renders $type background color', ({ type, color }) => {
const { getByRole } = render(<StyledGlobalAlertButton isPrimary $alertType={type} />);

expect(getByRole('button')).toHaveStyleRule('background-color', color);
});
Expand Down
31 changes: 19 additions & 12 deletions packages/notifications/src/styled/global-alert/StyledGlobalAlertButton.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import {
getColor,
DEFAULT_THEME,
focusStyles,
retrieveComponentStyles
retrieveComponentStyles,
ColorParameters
} from '@zendeskgarden/react-theming';
import { Button } from '@zendeskgarden/react-buttons';

Expand All @@ -24,7 +25,7 @@ interface IStyledGlobalAlertButtonProps {
isBasic?: boolean;
}

type OffsetOptions = Record<string, Record<string, number>>;
type OffsetOptions = Record<string, Record<string, ColorParameters['offset']>>;

function colorStyles(
props: IStyledGlobalAlertButtonProps & ThemeProps<DefaultTheme> & IStyledGlobalAlertButtonProps
Expand All @@ -36,32 +37,38 @@ function colorStyles(
}

let bgVariable;
let offsetOptions: OffsetOptions = { light: { offset: 200 }, dark: { offset: 300 } };
let offsetHoverOptions: OffsetOptions = { light: { offset: 300 }, dark: { offset: 400 } };
let offsetActiveOptions: OffsetOptions = { light: { offset: 400 }, dark: { offset: 500 } };
let offsetOptions: OffsetOptions;
let offsetHoverOptions: OffsetOptions;
let offsetActiveOptions: OffsetOptions;
Comment on lines +40 to +42
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let offsetOptions: OffsetOptions;
let offsetHoverOptions: OffsetOptions;
let offsetActiveOptions: OffsetOptions;
let offset: number;
let offsetHover: number;
let offsetActive: number;

...again, I think just using the getColor({ theme, offset: <number> }) is the better way to go, and easier to read/maintain.

let focusVariable;

switch ($alertType) {
case 'success':
bgVariable = 'background.successEmphasis';
offsetOptions = { light: { offset: 200 } };
offsetHoverOptions = { light: { offset: 300 } };
offsetActiveOptions = { light: { offset: 400 } };
focusVariable = 'foreground.successEmphasis';
break;
case 'error':
bgVariable = 'background.dangerEmphasis';
offsetOptions = { light: { offset: 200 } };
offsetHoverOptions = { light: { offset: 300 } };
offsetActiveOptions = { light: { offset: 400 } };
focusVariable = 'foreground.dangerEmphasis';
break;
case 'warning':
bgVariable = 'background.warningEmphasis';
focusVariable = 'foreground.warningEmphasis';
offsetOptions = { dark: { offset: 100 } };
offsetHoverOptions = { light: { offset: 100 }, dark: { offset: 200 } };
offsetActiveOptions = { light: { offset: 200 }, dark: { offset: 300 } };
offsetOptions = {};
offsetHoverOptions = { light: { offset: 100 } };
offsetActiveOptions = { light: { offset: 200 } };
focusVariable = 'foreground.warning';
break;
case 'info':
bgVariable = 'background.primaryEmphasis';
offsetOptions = { dark: { offset: 100 } };
offsetHoverOptions = { light: { offset: 100 }, dark: { offset: 200 } };
offsetActiveOptions = { light: { offset: 200 }, dark: { offset: 300 } };
offsetOptions = {};
offsetHoverOptions = { light: { offset: 100 } };
offsetActiveOptions = { light: { offset: 200 } };
focusVariable = 'foreground.primary';
break;
}
Expand Down
Loading