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 & some KButton stories #312

Closed

Conversation

nucleogenesis
Copy link
Member

Description

We've discussed in the past wanting to try out Storybook, particularly for KDS, and I had set it up with ease on a personal toy project a few months back so I have been optimistic about setting it up here and working with it.

  • Depends on using the version of kolibri-tools in Webpack 5 upgrade kolibri#9014 because Storybook requires Node >= 14 and kolibri-tools currently requires Node 10.
  • Initialized Storybook using npx sb init (very convenient, excellent install process)
  • Configured the Storybook preview.js config to bootstrap KThemePlugin (may still need to do the same in the main.js config file to accommodate building Storybook - still more to learn here)
  • Add dependencies for sass and sass-loader to devDependencies

Steps to test

  1. Be using Node 16
  2. In local Kolibri, checkout this PR Webpack 5 upgrade kolibri#9014
  3. cd into packages/kolibri-tools and run yarn link
  4. Go to your KDS directory, yarn link kolibri-tools
  5. yarn storybook should spin up the server

(optional) Implementation notes

Does this introduce any tech-debt items?

Story organization is something I'm not sure about. Dumping them all in one folder may not be easy to track over a long period of time but that's a problem we can figure out when we get there I suppose.

@nucleogenesis nucleogenesis added the TODO: needs decisions Further discussion and planning necessary label Feb 9, 2022
Copy link
Member

@MisRob MisRob left a comment

Choose a reason for hiding this comment

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

Thank you @nucleogenesis for your effort. I am not sure if we need Storybook in KDS and would suggest discussing the idea of using the KDS documentation site itself for this purpose in more detail at first. Posted more details on Slack.

Copy link
Member

@MisRob MisRob left a comment

Choose a reason for hiding this comment

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

To sum it up briefly, my main concern is that I don't see a single story for KButton here that we couldn't visualize and eventually test in our KDS documentation using regular Vue template syntax in DocsShow while improving the documentation itself on that opportunity which seems to be a promising alternative to Storybook to me, that I'd welcome to explore in more detail before introducing new tooling for everyone.

It seems that @bjester can see some benefits in Storybook, particularly in regards to testing, and has some ideas about how to overcome duplicate stories/examples (@bjester please correct me or specify more).

We can chat about it more for sure. It might take me a while to get to this again, and I don't want to block it for everyone, so if the majority prefers playing around with it rather sooner than later, feel free to go ahead and we can see how it works.

You can see this thread for details https://learningequality.slack.com/archives/CTG8XDTCL/p1644482179585449

@MisRob
Copy link
Member

MisRob commented Feb 10, 2022

Story organization is something I'm not sure about. Dumping them all in one folder may not be easy to track over a long period of time but that's a problem we can figure out when we get there I suppose.

Maybe having a file with stories for a component in a directory together with that component? It's similar to how Jest tests can be organized. I find it helpful since it's easy to navigate and I think we discussed having the same structure in Studio and Kolibri one day ideally in regards to Jest files (story file is similar to the spec file, in a way)

@nucleogenesis nucleogenesis marked this pull request as ready for review March 16, 2022 23:18
@nucleogenesis nucleogenesis requested a review from MisRob March 19, 2022 00:56
Copy link
Member

@MisRob MisRob left a comment

Choose a reason for hiding this comment

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

Thank you, @nucleogenesis, code looks good to me.

@nucleogenesis
Copy link
Member Author

I'm going to need to release kolibri-tools so that KDS can depend on it here because Storybook requires >=14 Node. I'll come back to this and take care of that later this week.

Copy link
Contributor

@sairina sairina left a comment

Choose a reason for hiding this comment

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

lgtm! I appreciate the instructions in the README, too. They are clear and informative.

@MisRob MisRob changed the base branch from main to develop June 22, 2022 13:03
@nucleogenesis
Copy link
Member Author

Noting that with @MisRob 's presentation of how to use KDS for the same purpose we'd use Storybook, this may not really be useful anymore. Thoughts @marcellamaki ?

@marcellamaki
Copy link
Member

@nucleogenesis - I think that's a good point. I don't think there's harm in having it set up, but I also don't think it's necessary. If you'd like to get this merged, I think we could - otherwise, okay with me to backburner this and close it.

@rtibbles
Copy link
Member

rtibbles commented Oct 6, 2022

I don't think there's harm in having it set up

One possible harm is that we now have 2 ways of doing something - and ideally editing the documentation is preferable, because then it's not only doing a live test of the component, but also creating documentation for it too?

@bjester
Copy link
Member

bjester commented Oct 6, 2022

Ah this conversation is coming back to me now. In the Slack thread @MisRob shared, I had an idea for how we could leverage Storybook stories in the KDS docs, that could also enrich our documentation. I imagined it would allow us to do something like the Shopify Polaris design system (try out the buttons at the top, and how it switches between different examples)

From the slack thread:

You made some good points here, Misha, but I also think we can have the benefits of Storybook without the maintenance downsides you've described. From what I can see, the Storybook stories at their core are simply functions that return a component to render. Whether it be with Storybook, defining Jest tests, or showing examples in the documentation, there needs to be some data structure that defines what should be rendered or tested. So with Storybook's approach, we have a definition structure that isn't tightly coupled with Storybook, meaning we could easily use the same information for displaying examples in the documentation and running tests (either Jest or browser-based). So for example:

  • A new KDS documentation component could take a list of stories for a single component, and render a dropdown for each story name, which on change, renders the component returned from the story. Then any stories we add can be used in Storybook and displayed in the documentation
  • For testing, each describe scenario maps nicely to each story, so for each of them we could use the stories for defining and mounting the component implementation scenario that's being tested, with some orchestration between the stories and test definitions. Also for browser-based tests and visual diffs, Storybook displays a story on a "canvas" which appears to load an iframe that takes a query parameter of the story, so it allows us to the do the same thing by loading the iframe URL.
  • We could cover the same testing scenarios in browser-based tests as we can in Jest. For CI, they could run in Chrome headless. But we're not quite there yet, and there won't be that many differences between these tests and Jest tests if we utilize the Storybook stories

@bjester
Copy link
Member

bjester commented Oct 18, 2022

Following up to this, I realized I have some lingering work in my KDS repo, something like an example along the lines of my last comment.

@MisRob
Copy link
Member

MisRob commented Jan 4, 2023

I had an idea for how we could leverage Storybook stories in the KDS docs, that could also enrich our documentation. I imagined it would allow us to do something like the Shopify Polaris design system (try out the buttons at the top, and how it switches between different examples)

@bjester Switching between different examples is something that'd need to be added to our KDS documentation system common components, no matter whether we use Storybook or not. You can see DocsPageTemplate, DocsShow, and other common docs components, and how they are used on actual documentation pages.

@MisRob
Copy link
Member

MisRob commented Jan 4, 2023

I second @rtibbles's comment:

One possible harm is that we now have 2 ways of doing something - and ideally editing the documentation is preferable, because then it's not only doing a live test of the component, but also creating documentation for it too?

Plus I'd add maintenance costs.

@bjester Regarding your proposal about using Storybook stories as a source for KDS stories: On a high level, writing stories in 3rd party system that uses a different syntax (see https://storybook.js.org/docs/vue/writing-stories/args#story-args) just to convert them to our own system is unnecessarily complicated in my view, when compared to writing them directly in Vue like we currently do, with the help of some common documentation components that are already implemented, work well, and can be extended to our needs. It would open relatively lots of issues affecting the way we work with KDS documentation that would need to be answered first. I can elaborate on it but before that, I think it'd be good to answer the most important question - do we need this, and why? If you want to move in such a direction, at first I'd suggest hearing from the team what benefits Storybook can offer us in the KDS context that the existing KDS documentation system can't. Not in a sense of its general features, but rather real things that we lack and want to start using on daily basis.

@bjester
Copy link
Member

bjester commented Jan 4, 2023

do we need this, and why?

@MisRob defining some information architecture for organizing examples is a good motivation, something that's common place with any code files, database tables, or test structures. If we add multiple examples, it seems unscalable to plop them directly into the documentation component along the lines of "writing them directly in Vue like we currently do." Utilizing an existing architecture that's simple and well documented like Storybook's means we can leverage their documentation for that architecture. Having a defined information architecture opens the door to further enhancements like I've already mentioned that bring real benefits that we lack. I view it as forward looking.

It would open relatively lots of issues affecting the way we work with KDS documentation that would need to be answered first.

What sorts of issues?

@indirectlylit
Copy link
Contributor

indirectlylit commented Jan 4, 2023

I've been lurking on this thread out of curiosity, and will briefly chime in with some context and thoughts. Of course, I fully respect what ever direction the current team chooses to take KDS in, and will probably not add much else here unless someone has a specific follow-up question.

When KDS was first being built I investigated both basing it fully on Storybook and also integrating Storybook as a way to show examples as discussed here. I also investigated Vue Styleguidist for similar purposes.

I eventually came to believe that:

  1. Storybook would be very useful for automated regression testing because of its ability to enumerate hard-to-reach states of components, e.g. error states, loading indicators, and long-text edge cases. It would pair very nicely with a perceptual/visual diff tool (like Percy or our abandoned "health inspector" CI tool) to catch layout and style regressions that are hard to check with code-based tests and hard to catch with manual QA. Most of these corner cases are not relevant to users reading documentation, only to people modifying the library components.
  2. My sense was that KDS users would benefit from fewer, more curated examples interspersed with explanations rather than extensive lists of examples. I came to believe that a "narrative" documentation section at the top (not just a list of examples) would be most useful for learning and explanation, and the API section at the bottom would serve as the reference. (This of course greatly oversimplifies the complex spectrum of user needs with technical docs, along with the additional complexity of catering to both designers and engineers...)
  3. Because the documentation pages use KDS components directly for its examples in the same way that Kolibri/Studio/etc do, there ought to be less cognitive overhead if a KDS user wants to help improve the documentation site.

Overall my recommendation would be to see whether more automated testing is worth the effort based on recent history of regressions. If so, storybook could be a useful engineering and CI tool to prevent these in the future. However I think it's less useful for user-facing documentation. That said, if the decision is made to integrate Storybook within the documentation, some details to watch out for:

  • Does the example syntax actually match (in a copy/paste sense) what would be used in external products? I encountered differences in component capitalization and prettier formatting because the components were compiled by Vue's in-browser compiler rather than through single-file components.
  • Ensuring the same versions of Vue, webpack loaders, etc are used everywhere. I ran into issues with this.
  • Loading our JS-based themes had some extra complexity
  • Handling of cross-links between storybook and nuxt pages to ensure a seamless UX both when using the dev server and after static site generation had some extra complexity

@indirectlylit
Copy link
Contributor

Oh - one other very important note:

🎉 Happy new year! 🎉

I miss you all... keep up the great work 😄

@MisRob
Copy link
Member

MisRob commented Jan 4, 2023

Good to still see you around from time to time @indirectlylit, thank you, and happy new year to you too and everyone who's reading this.

@bjester
Copy link
Member

bjester commented Jan 9, 2023

Thanks @indirectlylit for your thoughts! I did some short investigation into the visual diffs using Percy a few years ago, on that project you proposed for Kolibri. With that background, it seemed like utilizing Storybook for rendering individual examples and capturing screenshots for Percy could work really well. There are certainly other ways we could accomplish that, but I think extracting those screenshots from the documentation pages could be more subject to regressions if we make updates to documentation layouts and/or components. So in my thoughts, using some shared information architecture for the examples could support both independently while minimizing the chance and overhead of maintaining everything in the docs pages.

I think having something like Percy run on every PR, on examples connected to components that were modified in the PR, could be really valuable, let alone a really cool development tool.

@MisRob
Copy link
Member

MisRob commented Jan 11, 2023

it seems unscalable to plop them directly into the documentation component along the lines of "writing them directly in Vue like we currently do."

Perhaps a matter of opinion, I see this simplicity and flexibility as one of the best features of the current KDS documentation system. We typically don't need to write documentation with 20 examples at the top like in Shopify's Polaris. I appreciate the Jupyter Notebook feel of telling an interactive story. Another important benefit is that you can take exactly the same code from DocsShow and put it into DocsShowCode to demonstrate code for an example if you want to. This is one of the things that would need some research if we were to switch to Storybook, I believe.

What sorts of issues?

Mostly related to areas that @indirectlylit already mentioned above


I agree that automated testing would be very good. In an ideal world, we would be able to use the same source of data and follow general best practices. Often, things are not ideal and premature generalization can introduce complexity. I am suspicious that it may be the case with using Storybook for user documentation, even when just partially. Perhaps I'm wrong or maybe some more complexity would be worth it, I only suggest that before solutions or more discussions, we are very clear on what are the limitations we need to improve here as the needs for Storybook as a tool for automated testing are different from the needs for Storybook as documentation.

@MisRob
Copy link
Member

MisRob commented Jan 12, 2023

@nucleogenesis Just to be sure, I still think that this particular PR is okay to be merged if folks want to play around with Storybook, as noted when approving it couple of months ago. My recent comments about the need to discuss various motivations and research things are related to refactoring KDS to be built around it as documenation tool. Apologies if that wasn't clear. cc @bjester

@nucleogenesis
Copy link
Member Author

This PR needs to be fixed up a bit before merging and I've not come back to it in some time. I'm not sure when I'll fix it up but I think that it depends largely on the discussion around moving our documentation to use Storybook.

It's unclear to me what the path forward would look like and what changes would have to be made to fully leverage Storybook.

  • Would we just replace the examples in our existing bespoke docs components with a Vue component that just shows a specific story / set of stories?
  • Would all of the component-specific documentation be where we show every story even if we only show a select handful in the longer documentation pages?

I do think that, overall, we'll have a hard time prioritizing this given the many other things we have going right now, but I think it will be worth continuing to bear it in mind as we find ways to improve KDS and our dev experience with it because even without the bells and whistles of automated visual diff testing or whatever, it will be a straightforward experience to test changes made to components.

@MisRob
Copy link
Member

MisRob commented Apr 7, 2023

This PR is one example of work where Storybook for testing may help us: #429. The PR fixes KSelect's interaction with KModal and even though it's possible to create the testing environment as a documentation page, every reviewer would need to do it on their own, alternatively, it could be a part of the PR that would later be removed as it's not needed for documentation purposes. It'd be useful to keep the example somewhere so that we can test for regressions after future updates of models and selects. I imagine this may be the case generally for when we need to ensure seamless interaction of KDS components.

@MisRob
Copy link
Member

MisRob commented Oct 12, 2023

This will need to be rebased and retargeted to main branch

@nucleogenesis
Copy link
Member Author

Closing. We can revisit this if/when the topic comes up again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO: needs decisions Further discussion and planning necessary
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants