search

torchbox / django-pattern-library

UI pattern libraries for Django templates
https://torchbox.github.io/django-pattern-library/
BSD 3-Clause "New" or "Revised" License
359 stars 43 forks source link

storybook-django: Storybook prototype of the pattern library #103

Open thibaudcolas opened 4 years ago

thibaudcolas commented 4 years ago

Tracking issue for conversations relating to storybook-django, a prototype Storybook integration of the pattern library.

Related issues

All of the issues tagged storybook are related to this integration – either because the Storybook UI addresses them, or because using the pattern library through such an integration would drastically change the implementation.

Addressed by Storybook

Here are open issues that would no longer be relevant when using the pattern library with Storybook:

And issues for which the implementation would be drastically simpler:

thibaudcolas commented 4 years ago

Storybook prototype review

We’ve reviewed this together with @bcdickinson and @William-Blackie today – here are things to be addressed / to think of further,

And clear improvements that this brings:

The takeaway from our conversation is that this comes with clear improvements on multiple fronts (UI, workflow, constraints on file structure), and doesn’t make other areas of the pattern library worse, beyond the initial setup.

There’s more for us to research (cc @bcdickinson) on whether this would make it easier for us to address the pattern library’s limitation to basic data structures (as opposed to Python-specific types / Django objects), and whether it could help address the limitation of having to hard-code a lot of mock data (integration with factories).

@bcdickinson I think this would be a good place to share your findings once you’ve had the time to look into this more.

thibaudcolas commented 4 years ago

I’ve experimented with this a bit more since writing the last comment and found something that seems to work quite well – automatically generating stories based on the project’s YAML files.

There is a barebones example of this at https://github.com/torchbox/storybook-django/blob/master/demo/storybook/legacy.stories.js, and here is a more advanced example:

View code ```jsx import { storiesOf } from '@storybook/react'; import React from 'react'; import { text, number } from '@storybook/addon-knobs'; import { TemplatePattern } from 'storybook-django'; import '../../project_styleguide/templates/patterns/base.html'; import '../../project_styleguide/templates/patterns/base_page.html'; const req = require.context( '../../project_styleguide/templates/patterns', true, /\.yaml$/, ); const knobify = (obj) => { return Object.keys(obj).reduce((cont, key) => { const val = obj[key]; if (typeof val === 'string') { cont[key] = text(key, val); } else if (typeof val === 'number') { cont[key] = number(key, val); } else { cont[key] = val; } return cont; }, {}); }; req.keys().forEach((path) => { /* eslint-disable global-require, import/no-dynamic-require */ const yaml = req(path); const cleanPath = path.replace('./', '').replace('.yaml', ''); const pathElts = cleanPath.split('/'); const filename = pathElts.pop(); const source = require(`../../project_styleguide/templates/patterns/${cleanPath}.html`); const rawYaml = require(`!!raw-loader!../../project_styleguide/templates/patterns/${cleanPath}.yaml`); const context = yaml?.context ?? {}; const usage = Object.keys(context) .map((key) => { const val = context[key]; const wrappedVal = typeof val === 'string' ? `"${val}"` : val; return `${key}=${wrappedVal}`; }) .join(' '); const folders = pathElts.join('/') || 'Templates'; storiesOf(folders, module).add( filename, () => { return ( ); }, { notes: { markdown: ` ### Usage (experimental) \`\`\`html\n{% include "patterns/${cleanPath}.html" with ${usage} %}\n\`\`\` ### Source \`\`\`html\n${source.default}\n\`\`\` ### YAML \`\`\`yaml\n${rawYaml.default}\n\`\`\``, }, }, ); }); ```

This has a number of advantages:

Doing this, I also realised a few shortcomings of the prototype:

thibaudcolas commented 4 years ago

Here’s a quick GIF demo of this setup:

storybook-django

chris-lawton commented 4 years ago

I've added this to BCUK (although have not made an MR based on this - I can do if that would be useful) and have had a good look around. My findings:

Good 👍

Observations 👀

Thoughts 🤔

thibaudcolas commented 4 years ago

Thank you @chris-lawton 🤘 This is very useful feedback! I’ve removed links straight to the project’s code from your comment. I don’t think this is particularly problematic but since this is all public I’d rather be extra cautious. Might be worth raising with the sysadmin whether this kind of caution is warranted or not.

For JS – I suspect this is just because JS for each component isn’t loaded in the Storybook version. This is something I didn’t implement with the sample code I sent you. The vanilla pattern library renders each component as if they were inside the project’s base template: https://github.com/torchbox/django-pattern-library/tree/master/docs#customising-the-patterns-surroundings

…which (for most builds) includes the project’s stylesheet, JS, SVG icons. For the Storybook version, by default patterns are rendered directly without any base template, so we need to load the CSS & JS manually, as well as any other component dependencies. Inside the Storybook preview.js:

import { configure, addDecorator, addParameters } from '@storybook/react';
import { withA11y } from '@storybook/addon-a11y';

import iconSprite from '../../project_styleguide/templates/patterns/atoms/sprites/sprites.html';

configure(() => {
    const hasIcons = document.querySelector('[data-storybook-svg-icons]');

    if (!hasIcons) {
        const icons = document.createElement('div');
        icons.setAttribute('data-storybook-svg-icons', true);
        icons.innerHTML = iconSprite;
        document.body.appendChild(icons);
    }

    // eslint-disable-next-line global-require
    require('../sass/main.scss');
}, module);

When I get back to this I’ll check what would be the best way to load JS, whether it’s as simple as require('../js/main.js');, or whether we’d want to do something else.

thibaudcolas commented 2 years ago

Here is a short update on this prototype.

TL;DR;

It seems to be working very well. I personally feel more productive with it than with DPL’s YAML files.

Highlights

Compared with django-pattern-library re the above comments,

import { Pattern } from 'storybook-django/src/react';

export default {};

export const Base = () => <Pattern filename={__filename} />;

This is the equivalent of not having a .yaml file with DPL. So 3 lines is still more than 0 line 0 file, but not as big as it used to be.

Known issues

There is only one I’m aware of compared to django-pattern-library – the current API to render a template only supports passing context and tags for the currently-rendered template, not any of its dependencies. For vanilla DPL this is only an issue in a few cases (#138, #8).

Next steps

I’m interested in trying out a version of DPL with the Storybook UI but without the runtime dependencies. This is dependent on the release of Storybook v7, as I want to make sure to not rely on any internal or deprecated APIs that might get removed (storiesOf API vs. CSF).

thibaudcolas commented 2 years ago

The prototype has been further trialed by @stevedya – we ran into #138, #193, and found an unrelated django-pattern-library bug: #199.

We tried this with Storybook v6.5, which is much lighter than previous releases. The next release, v7.0, promises to be even lighter, which might alleviate some of the concerns with using npm dependencies for this in addition to the Python django-pattern-library.

This was done on the Wagtail project, which uses storybook-django. Here is a PR showing the work involved: https://github.com/wagtail/wagtail/pull/8665.