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.

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

Replace theme add-on with toolbar and global args #1219

Merged
merged 8 commits into from
May 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion .storybook/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ module.exports = {
},
'@storybook/addon-a11y',
'storybook-mobile',
'storybook-addon-themes',
'storybook-addon-paddings',
'@whitespace/storybook-addon-html',
],
Expand Down
29 changes: 24 additions & 5 deletions .storybook/preview.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Parser } from 'html-to-react';
import { withPaddings } from 'storybook-addon-paddings';
import { INITIAL_VIEWPORTS } from '@storybook/addon-viewport';
import { withTheme } from './theme-decorator';
import tokens from '../src/compiled/tokens/js/tokens';
import 'focus-visible';
import '../src/index-with-dependencies.scss';
Expand Down Expand Up @@ -37,10 +38,6 @@ const breakpointViewports = Object.keys(breakpoints).map((name) => {
const htmlToReactParser = new Parser();

export const parameters = {
// Theme selection from stories
themes: [
{ name: 'Dark', class: 't-dark', color: tokens.color.brand.primary.value },
],
options: {
storySort: {
method: 'alphabetical',
Expand Down Expand Up @@ -75,4 +72,26 @@ export const parameters = {
paddings,
};

export const decorators = [withPaddings];
export const globalTypes = {
theme: {
name: 'Theme',
description: 'Global theme for components',
toolbar: {
icon: 'paintbrush',
items: [
{
// 'null' value supports a "no value selected" state, if 'undefined'
// there are sometimes missing 'key' errors in console
value: null,
title: 'No theme',
},
{
value: 't-dark',
title: 'Dark',
},
],
},
},
};

export const decorators = [withPaddings, withTheme];
65 changes: 65 additions & 0 deletions .storybook/theme-decorator.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { useEffect } from '@storybook/client-api';

/**
* Removes current theme classes on an element and applies a new theme class
* if provided.
* @param {HTMLElement} element - Element to add/remove theme classes from
* @param {string} [theme] - New theme to apply.
*/
const updateTheme = (element, theme) => {
if (!element || !element.classList) {
return;
}

const themes = [];
element.classList.forEach((className) => {
if (className.startsWith('t-') && className !== theme) {
themes.push(className);
}
});

if (themes.length > 0) {
element.classList.remove(...themes);
}

if (theme) {
element.classList.add(theme);
}
};

/**
* Adds theme class to containing story elements.
*
* This unfortunately does not work with non-inline stories in Docs view due to
* some open Storybook issues, specifically
* {@link https://github.com/storybookjs/storybook/issues/14477|#14477} and
* {@link https://github.com/storybookjs/storybook/issues/13444|#13444}.
*
* @param {function} story
* @param {object} context
* @returns {*} Result of story function.
*/
export const withTheme = (story, context) => {
const theme = context.parameters.theme || context.globals.theme;

if (context.viewMode === 'story') {
// In canvas view, theme the root HTML element
useEffect(() => {
updateTheme(document.body, theme);
});
} else if (context.viewMode === 'docs') {
useEffect(() => {
// Remove any existing theme classes from the root element
updateTheme(document.body);
// Query for the most appropriate story element
const storyElement = document.querySelector(`#story--${context.id}`);
// Only proceed if we actually found the relevant story.
if (storyElement) {
const previewElement = storyElement.closest('.docs-story');
updateTheme(previewElement, theme);
Comment on lines +58 to +59
Copy link
Member

Choose a reason for hiding this comment

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

I don't yet have an understanding of story elements and their contents. Is it a guaranty that there will always exist a .docs-story or do we need to check that previewElement is not null before attempting to update the theme? 🤔 ¯\_(ツ)_/¯

Copy link
Member

Choose a reason for hiding this comment

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

This seems like a good safety check given that the underlying Storybook markup is outside our control

Copy link
Member Author

Choose a reason for hiding this comment

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

The updateTheme function does check that the element is truthy and has a classList property, so I don't think another check before calling it is necessary.

}
});
}

return story();
};
33 changes: 0 additions & 33 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@
"sass": "1.32.13",
"sass-loader": "10.1.1",
"storybook-addon-paddings": "4.0.0",
"storybook-addon-themes": "6.1.0",
"storybook-mobile": "0.1.31",
"style-dictionary": "2.10.3",
"style-loader": "2.0.0",
Expand Down
8 changes: 2 additions & 6 deletions src/components/button/button.stories.mdx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { Story, Canvas, Meta } from '@storybook/addon-docs/blocks';
import elementsDemo from './demo/elements.twig';
import stylesDemo from './demo/styles.twig';
import stylesDarkDemo from './demo/styles-dark.twig';

<Meta
title="Components/Button"
Expand Down Expand Up @@ -35,10 +34,7 @@ Some buttons may not require as much emphasis as others. The `c-button--secondar
All the above visual styles will appear slightly different within [our dark theme](/?path=/docs/themes--dark). These versions retain their visual emphasis relative to one another while remaining visually distinct from form inputs.

<Canvas>
<Story
name="Styles (Dark)"
parameters={{ themes: { disable: true }, paddings: { disabled: true } }}
>
{(args) => stylesDarkDemo(args)}
<Story name="Styles (Dark)" parameters={{ theme: 't-dark' }}>
tylersticka marked this conversation as resolved.
Show resolved Hide resolved
{(args) => stylesDemo(args)}
</Story>
</Canvas>
5 changes: 0 additions & 5 deletions src/components/button/demo/styles-dark.twig

This file was deleted.

16 changes: 0 additions & 16 deletions src/design/demo/fill-canvas.twig

This file was deleted.

54 changes: 25 additions & 29 deletions src/design/demo/theme.twig
Original file line number Diff line number Diff line change
@@ -1,29 +1,25 @@
{% extends '@cloudfour/design/demo/fill-canvas.twig' %}

{% block content %}
{% embed '@cloudfour/objects/rhythm/rhythm.twig' with {
class: 'o-rhythm--generous'
} %}
{% block content %}
<h1>t-{{theme}}</h1>
{% include '@cloudfour/design/typography/demo/inline-elements.twig' only %}
{% include '@cloudfour/components/button/demo/styles.twig' only %}
{# TODO: Replace padding and corner modifiers once card component can
handle these variations natively #}
{% set _card_class = 'c-card--horizontal@m u-pad-1 u-rounded' %}
{% if theme == 'light' %}
{% set _card_class = _card_class ~ ' t-dark' %}
{% else %}
{% set _card_class = _card_class ~ ' t-light' %}
{% endif %}
{% include '@cloudfour/components/card/demo/single.twig' with {
class: _card_class,
href: '#',
show_heading: true,
show_content: true,
show_cover: true,
show_footer: true
} only %}
{% endblock %}
{% endembed %}
{% endblock %}
{% embed '@cloudfour/objects/rhythm/rhythm.twig' with {
class: 'o-rhythm--generous'
} %}
{% block content %}
<h1>t-{{theme}}</h1>
{% include '@cloudfour/design/typography/demo/inline-elements.twig' only %}
{% include '@cloudfour/components/button/demo/styles.twig' only %}
{# TODO: Replace padding and corner modifiers once card component can
handle these variations natively #}
{% set _card_class = 'c-card--horizontal@m u-pad-1 u-rounded' %}
{% if theme == 'light' %}
{% set _card_class = _card_class ~ ' t-dark' %}
{% else %}
{% set _card_class = _card_class ~ ' t-light' %}
{% endif %}
{% include '@cloudfour/components/card/demo/single.twig' with {
class: _card_class,
href: '#',
show_heading: true,
show_content: true,
show_cover: true,
show_footer: true
} only %}
{% endblock %}
{% endembed %}
22 changes: 9 additions & 13 deletions src/design/themes.stories.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,7 @@ import { Meta, Story, Canvas } from '@storybook/addon-docs/blocks';
import demo from './demo/theme.twig';
const demoStory = (args) => demo(args);

<Meta
title="Design/Themes"
parameters={{ themes: { disable: true }, paddings: { disabled: true } }}
argTypes={{
theme: {
type: { name: 'string' },
control: { type: 'inline-radio', options: ['light', 'dark'] },
defaultValue: 'light',
},
}}
/>
<Meta title="Design/Themes" />

# Themes

Expand All @@ -23,15 +13,21 @@ Themes may be applied to a containing element to change that content's appearanc
The default theme, applied to the document `:root` by default.

<Canvas>
<Story name="Light">{demoStory.bind({})}</Story>
<Story
name="Light"
parameters={{ theme: 't-light' }}
args={{ theme: 'light' }}
>
{demoStory.bind({})}
</Story>
</Canvas>

## Dark

Optimized for page intros and headings.

<Canvas>
<Story name="Dark" args={{ theme: 'dark' }}>
<Story name="Dark" parameters={{ theme: 't-dark' }} args={{ theme: 'dark' }}>
tylersticka marked this conversation as resolved.
Show resolved Hide resolved
{demoStory.bind({})}
</Story>
</Canvas>
Expand Down