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

Storybook: Addon to wrap stories in max-width div #45134

Merged
merged 2 commits into from
Oct 24, 2022
Merged

Storybook: Addon to wrap stories in max-width div #45134

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e7aef6d
Storybook: Addon to wrap stories in max-width div
aaronrobertshaw Oct 20, 2022
a04734f
Rename and clean up max-width addon
aaronrobertshaw Oct 20, 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
29 changes: 29 additions & 0 deletions storybook/decorators/with-wrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* External dependencies
*/
import styled from '@emotion/styled';

/**
* A Storybook decorator to wrap a story in a div applying a max width and
* padding. This can be used to simulate real world constraints on components
* such as being located within the WordPress editor sidebars.
*/

const Wrapper = styled.div`
max-width: 248px;
padding: 16px;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also show an outline ? It would help to read the padding better

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 did ponder adding a border myself but erred towards minimising changes from how the current component stories looked. I'll add one tomorrow 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

For sure there are pros and cons — I though an outline / border could help also with the look of the sidebar?

Copy link
Member

Choose a reason for hiding this comment

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

A couple small things:

  1. I believe the correct metrics are 280-(16*2) = 248px? (248px is the internal content width, after padding)
  2. I'm hoping we can keep this add-on to have a singular purpose for now — so, just max-width and nothing else. (No padding, no borders, etc). Would that be ok?
  3. One of the reasons for point 2 ☝️ is that almost every Storybook decorator we have is a "wrapper". So it would be good to be clear about what each wrapper is for. Could we change all the naming to be more specific, like WithMaxWidthWrapper, context.globals.maxWidthWrapper, etc?

We could also consider injecting an indicator div (like the margin checker) so nobody gets confused whether or not a max-width is being applied. No strong opinion on this at the moment though. We could add something later if we do find it confusing.

Example of an indicator div to show that there's a max-width wrapper

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, all good suggestions.

  • Padding has been removed
  • Max width has been corrected
  • No border added
  • Add-on has been renamed to indicate a singular purpose
  • Indicator has also been added
    • Styles copied from the margin checker
    • I think it's good to make it easy to know what max-width is being applied

`;

export const WithWrapper = ( Story, context ) => {
if ( context.globals.wrapper === 'none' ) {
return <Story { ...context } />;
}

return (
<>
aaronrobertshaw marked this conversation as resolved.
Show resolved Hide resolved
<Wrapper>
<Story { ...context } />
</Wrapper>
</>
);
};
14 changes: 14 additions & 0 deletions storybook/preview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { WithGlobalCSS } from './decorators/with-global-css';
import { WithMarginChecker } from './decorators/with-margin-checker';
import { WithRTL } from './decorators/with-rtl';
import { WithTheme } from './decorators/with-theme';
import { WithWrapper } from './decorators/with-wrapper';
import './style.scss';

export const globalTypes = {
Expand Down Expand Up @@ -63,13 +64,26 @@ export const globalTypes = {
],
},
},
wrapper: {
name: 'Wrapper',
description: 'Wrap the component in a div with a max-width.',
defaultValue: 'none',
toolbar: {
icon: 'outline',
aaronrobertshaw marked this conversation as resolved.
Show resolved Hide resolved
items: [
{ value: 'none', title: 'None' },
{ value: 'wordpress-sidebar', title: 'WP Sidebar' },
],
},
},
};

export const decorators = [
WithTheme,
WithGlobalCSS,
WithMarginChecker,
WithRTL,
WithWrapper,
];

export const parameters = {
Expand Down