Skip to content

Conversation

@ChristopherChudzicki
Copy link
Contributor

@ChristopherChudzicki ChristopherChudzicki commented Jun 28, 2024

What are the relevant tickets?

Description (What does it do?)

This PR:

  1. fixes publishing storybook on github pages. It wasn't working before
    • it will only publish when manually run or when there are pushes to main.
  2. Removes our github-pages react app.
    • At one point in the past, we had a react app + storybook deploying to github pages. The react app didn't have much besides a link to storybook, so we've just removed it.
  3. Renames a few storybook stories.
    • Previously we had two "Categories": ol-components/storyname and smoot-design/storyname. I've moved most things under ol-components/ to smoot-design/ (this only affects the storybook, not the workspace name) and renamed ol-components category to "old"

How can this be tested?

  1. Run yarn storybook locally and compare with https://mitodl.github.io/mit-open/. They should be the same.

"html-webpack-plugin": "^5.6.0",
"mini-css-extract-plugin": "^2.6.1",
"ol-test-utilities": "0.0.0",
"react-refresh": "^0.14.2",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a peer-dependency of another package we use, @pmmmwh/react-refresh-webpack-plugin. Previously we were not installing the peer dep explicitly (it was added by react-scripts, I believe). Now that the github-pages workspace is removed, we needed to explicitly install it.

@ChristopherChudzicki ChristopherChudzicki changed the title try separate job Fix storybook github pages publishing Jul 1, 2024
@ChristopherChudzicki ChristopherChudzicki added the Needs Review An open Pull Request that is ready for review label Jul 1, 2024
@ChristopherChudzicki ChristopherChudzicki marked this pull request as ready for review July 1, 2024 16:14
on:
# Runs on pushes targeting the default branch
push:
branches: [$default-branch, cc/publish-pages]
Copy link
Contributor

@jonkafton jonkafton Jul 2, 2024

Choose a reason for hiding this comment

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

Remove PR branch for cleanup

with:
path: ./frontends/mit-open/storybook-static

# Deploy job
Copy link
Contributor

Choose a reason for hiding this comment

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

These comments aren't adding much. Perhaps not doing any harm; I'd remove them.

@jonkafton
Copy link
Contributor

PR looks good. Now we have static hosting for the front end set up, let's pick up the conversation of publishing Storybook alongside it for branch PRs and environments.

For the Storybook renaming, do you think it's worth making the distinction between components that form part of the design language - the Smoot Design components - and components that are just UI devices without any particular design - e.g. PlainList, Popover, the RoutedDrawer (arguably)?

@jonkafton jonkafton removed the Needs Review An open Pull Request that is ready for review label Jul 2, 2024
Copy link
Contributor

Choose a reason for hiding this comment

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

Small nit - the other workflow filenames are hyphened.

@jonkafton
Copy link
Contributor

Might be worth updating the project README (last line) to say it's now publishing Storybook there.

@ChristopherChudzicki
Copy link
Contributor Author

@jonkafton

For the Storybook renaming, do you think it's worth making the distinction between components that form part of the design language - the Smoot Design components - and components that are just UI devices without any particular design - e.g. PlainList, Popover, the RoutedDrawer (arguably)?

I do think that this is a good idea and worthwhile, but mostly I want to get this deployed and shareable with Steve/Simone. I suspect we will do some further reorganizing / renaming after they've looked at it a bit. I'd like to hold off on more reorganizing till then.

@ChristopherChudzicki ChristopherChudzicki merged commit efc959b into main Jul 2, 2024
This was referenced Jul 8, 2024
@rhysyngsun rhysyngsun deleted the cc/publish-pages branch February 7, 2025 20:31
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