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

Add theme switching to Storybook #2326

Merged
merged 3 commits into from
Jan 26, 2024

Conversation

lindapaiste
Copy link
Collaborator

Changes:

  • Add a "Theme" option to Storybook controls.
  • Create a decorator which applies the selected theme to the story.
  • Default view is all 3 themes together, but can also select a single theme.

image
image
image

Could be improved if I make the background fill the entire preview area.

@ofhope - thoughts?

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

@lindapaiste
Copy link
Collaborator Author

Trying out this feature with all of the new stories added in #2398 has revealed a bunch of things which are not handled perfectly.

TODOs:

  • Some SVGs (ex. EditableInput pencil) don't work due to a lack of viewbox. I need to look into the svgr plugin settings.
  • Elements which use fixed or absolute positioning (ex. modals, FloatingActionButton) will show up on top of each other when viewing all themes at once.
  • File uploader does not work in all-theme mode due to using an element id to initialize the dropzone, as this id is not unique when three versions are rendered at once.
  • Modals aren't getting the correct text color in the dark and contrast themes. I suspect they inherit this from some parent when in the app?

@ofhope
Copy link
Contributor

ofhope commented Sep 15, 2023

This is awesome! Its really nice to be able to isolate and explore components with a theme.

The all view with three together may be questionable. It doesn't play so nice with more complex stories, Modal like things. It'd be great for primitives (button, input, icon etc). But if more themes become available it could clutter the story (probably unlikely that more themes will come just speculating here).

One idea for a view with all themes is to mimic Compositions https://bit.cloud/teambit/design/inputs/select

Screenshot 2023-09-15 at 10 06 14 am

Storybook has MDX support so you could use your all view in an MDX document to print out all view variations in a supporting document. This would leave the stories as singular views and allow control over which components had a supporting MDX file with an all theme view.

But these are just ideas for the future. I think whats here is an enhancement by exposing the theme in the first place. It can be iterated on in the future if needed.

@@ -21,5 +21,9 @@ export const decorators = [
</MemoryRouter>
Copy link
Contributor

@ofhope ofhope Sep 15, 2023

Choose a reason for hiding this comment

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

It looks like ThemeProvider has moved to the withThemeProvider HOC. Might need to remove <ThemeProvider> as well.

@raclim raclim added this to the MINOR Release for 2.12.0 milestone Jan 26, 2024
@raclim raclim merged commit 2b99345 into processing:develop Jan 26, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants