Skip to content

docs: fix storybook #2388

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

Merged
merged 2 commits into from
Aug 18, 2023
Merged

Conversation

ofhope
Copy link
Contributor

@ofhope ofhope commented Aug 17, 2023

Fixes storybook links and css

So comparing some of the styles in ErrorModal I can see they're in SCSS but weren't in build/main.css. It looks like this file hasn't been regenerated in a while and isn't used.

  • Removed build/main.css
  • Added a command to trigger scss to css transpiling
  • Added a .gitignore file to ignore the css generated for storybook previews
  • Moved the built css file up so the images are resolvable relative to the css file (the svgs break the build if the css doesn't sit in a sibling directory)
  • And lastly ErrorModal has some links which started breaking so wrapped stories in a MemoryRouter

Note: it probably is solveable to just load the main.scss file. Storybook have some documentation on it. https://storybook.js.org/recipes/sass But this sass package isn't currently used. I had a quick go at attempting to include sass-loader in the storybook webpack config but I was unsuccessful. For now this seems like the simplest approach.

Screenshot 2023-08-17 at 10 33 34 pm Screenshot 2023-08-17 at 10 33 38 pm Screenshot 2023-08-17 at 10 33 44 pm

Changes:

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

Copy link
Collaborator

@lindapaiste lindapaiste left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

Looks like the package that we are using for compiling sass in the main web app is node-sass which is actually deprecated. At some point we probably want to switch over to sass (Dart Sass) and we can revisit this when we do.

For now it seems like you've managed to find a good workaround.

@@ -0,0 +1 @@
storybook.css
Copy link
Collaborator

Choose a reason for hiding this comment

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

Love this! ❤️ We definitely don't want to include temporary/autogenerated files in the repo.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@raclim I'm 99% sure that this file is junk. It's weird to have a nested /build/ folder like this. Let me know if you are aware of any reason that we cannot delete this.

@lindapaiste lindapaiste merged commit 4bae3e1 into processing:develop Aug 18, 2023
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.

2 participants