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

docs(storybook): Theming support for stories #1151

Merged
merged 1 commit into from
Oct 30, 2024
Merged

Conversation

mcslayer
Copy link
Contributor

@mcslayer mcslayer commented Oct 28, 2024

Done

  • Hide default background select
  • Add global theme select Light/Dark/Paper for strorybook
  • Add HOC decorator for all stories
  • Add special hack for docs background

QA

Storybook

To see rendered examples of all react-components, run:

yarn start

QA in your project

from react-components run:

yarn build
npm pack

Install the resulting tarball in your project with:

yarn add <path-to-tarball>

QA steps

  • Steps for QA.

Percy steps

  • List any expected visual change in Percy, or write something like "No visual changes expected" if none is expected.

Fixes

Fixes: # .

@webteam-app
Copy link

mcslayer is not a collaborator of the repo

@bartaz
Copy link
Member

bartaz commented Oct 28, 2024

@bartaz bartaz changed the title Theming support for stories docs(storybook): Theming support for stories Oct 28, 2024
@bartaz
Copy link
Member

bartaz commented Oct 28, 2024

This is awesome addition @mcslayer!

We have a requirement for all PRs to be up to date with main branch, so please rebase now that your Vanilla update is merged, thx!

@bartaz
Copy link
Member

bartaz commented Oct 28, 2024

Is there some kind of storybook documentation or recommendation that you followed when adding the theme switcher?

@mcslayer
Copy link
Contributor Author

mcslayer commented Oct 28, 2024

There is no ready-made special documentation for this. The code is based on personal experience of using storybook in projects.

@mcslayer
Copy link
Contributor Author

mcslayer commented Oct 28, 2024

Also, instead of "Theme" we can write the name of the active theme and It looks even better. So I can add it if you approve.

@bartaz
Copy link
Member

bartaz commented Oct 28, 2024

Also, instead of "Theme" we can write the name of the active theme and It looks even better. So I can add it if you approve.

Nice! If possible, I think it would be better for it to say both Theme: Paper Just to make sure people understand what they are changing (As "paper" itself may be not self explanatory).

(sorry, I updated you comment by mistake instead of replying 🤦 )

@mcslayer
Copy link
Contributor Author

I made the order in the commits and made 1 commit with a force push.

Regarding the title: everything is primitive there. You can use the abstract "Theme", or take the title from the options. So I changed the titles everywhere in the list to Theme: Light, Theme: Dark, Theme: Paper

@mcslayer
Copy link
Contributor Author

If you apply this global control, I'm preparing something else :)

ezgif-7-2bdfc4dc2e

Copy link
Member

@bartaz bartaz left a comment

Choose a reason for hiding this comment

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

Great addition, thank you!

@bartaz bartaz merged commit 82f0d89 into canonical:main Oct 30, 2024
8 checks passed
@mcslayer mcslayer deleted the theming branch October 30, 2024 20:12
Copy link

github-actions bot commented Nov 1, 2024

🎉 This PR is included in version 1.7.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants