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

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

Open
thibaudcolas opened this issue Feb 28, 2020 · 7 comments
Open

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

thibaudcolas opened this issue Feb 28, 2020 · 7 comments
Labels
enhancement New feature or request storybook Usage of Storybook will affect this ui Related to the user interface of the pattern library
Milestone

Comments

@thibaudcolas
Copy link
Member

thibaudcolas commented Feb 28, 2020

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 thibaudcolas added enhancement New feature or request storybook Usage of Storybook will affect this ui Related to the user interface of the pattern library labels Feb 28, 2020
@thibaudcolas thibaudcolas added this to the Nice to have milestone Feb 28, 2020
@thibaudcolas
Copy link
Member Author

Storybook prototype review

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

  • Live reloading has no awareness of template inheritance and inclusion of Django Templates (extends, include, include_block)
  • Not much better off than YAML files from the perspective of writing Python/Django-friendly templates (using data structures beyond what serialises as JSON, e.g. QuerySets)
  • More things to npm install is annoying
  • More boilerplate to create stories (doesn’t seem like that big of a deal)
  • React API vs plain JS to create stories – to be decided

And clear improvements that this brings:

  • Support for pattern variants
  • UI improvements
  • No constraints on how to structure patterns
  • Faster dev workflow (built-in hot reload of templates only)
  • Ability to reuse Storybook’s ecosystem of plugins (e.g. accessibility checks, knobs, documentation generation)
  • React-friendly, so we can use the same tool regardless of how the UI is built

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
Copy link
Member Author

thibaudcolas commented Jun 18, 2020

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
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 (
                <TemplatePattern
                    template={`patterns/${cleanPath}.html`}
                    context={knobify(context)}
                    tags={yaml.tags}
                />
            );
        },
        {
            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:

  • From a migration perspective, we can keep on writing YAML files the same way as before, and get the benefits of Storybook without having to write JS stories (which is possible, but the API is still in flux)
  • It solves the "live reloading template inheritance problem" by just loading the project’s base templates by default. We could make this more robust though.
  • Additionally the fact that we’re generating stories from YAML allows us to generate a bit more than a barebones story – the example above also generates knobs for live-editing of the stories’ contents, and an experimental {% include %} pattern for the component.

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

  • Currently, template tag overrides are still YAML-only. I thought it worked via API rendering before, but django-pattern-library actually reads the YAML file separately for the template context and for the tags overrides. This isn’t a problem with the "middle ground" approach described above though.
  • Currently Storybook stories don’t load any JavaScript. So components relying on JS to display won’t work. I don’t think this is a fundamental problem, just something I haven’t needed yet so didn’t add.

@thibaudcolas
Copy link
Member Author

Here’s a quick GIF demo of this setup:

storybook-django

@thibaudcolas thibaudcolas self-assigned this Jun 23, 2020
@chris-lawton
Copy link
Member

chris-lawton commented Jun 30, 2020

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 👍

  • The components are in alphabetical order! Small i know, but so so useful.
  • The hot reload feels a lot quicker compared to django pattern library. Again, not a biggie but it all adds up.
  • No page reload when navigating patterns - navigating between patterns in DPL feels so much slower now.

Observations 👀

  • The JS for some components wasn't working as expected. For example the Accordion (patterns/molecules/accordion/accordion.html) component will toggle open/closed when viewing in DPL but just reloads the page in Storybook Django. Could be related the JS for the component (static_src/javascript/components/accordion.js) itself.

Thoughts 🤔

  • Being able to use Addons out of the box will save us so much time - we'd have to invest time in developing these ourselves if we ever wanted that functionality in DPL. (For perspective it's taken a while to just get a 'Docs' tab added - open sourcing this may help that somewhat)
  • Storybook has an active community and doesn't feel like it's going away anytime soon.

@thibaudcolas
Copy link
Member Author

thibaudcolas commented Jul 2, 2020

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
Copy link
Member Author

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

  • I’ve recently released a new version of storybook-django with much better APIs, and documentation.
  • In particular this comes with much better step-by-step "getting started" instructions, and a features comparison table with django-pattern-library.
  • The storybook-django demo site has been updated with a bunch of official addons, automated accessibility and visual regression testing tests, and E2E interaction tests.
  • We’re now using storybook-django in Wagtail.

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

  • Writing stories as YAML is still supported if that’s desirable or as a migration step. See demo.
  • The boilerplate for React-based stories is much smaller now, 3 lines:
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
Copy link
Member Author

thibaudcolas commented Jul 21, 2022

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: wagtail/wagtail#8665.

@thibaudcolas thibaudcolas changed the title Storybook prototype of the pattern library storybook-django: Storybook prototype of the pattern library Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request storybook Usage of Storybook will affect this ui Related to the user interface of the pattern library
Projects
No open projects
Development

No branches or pull requests

2 participants