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

Publish storybook to a public github page on every change in components or stories #413

Closed
wants to merge 8 commits into from

Conversation

Sebastian-ubs
Copy link
Contributor

@Sebastian-ubs Sebastian-ubs commented Sep 1, 2023

Fixes paranext/ux#3


This change is Reviewable

@Sebastian-ubs
Copy link
Contributor Author

Please set the following setting in repo settings:
Pages > branch: github-pages (and keep root)
Like here https://budiirawan.com/images/2022/storybook-github-pages-setting.jpg

Currently gh action config is pointing to this branch, to try it out.

Copy link
Member

@tjcouch-sil tjcouch-sil 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 the hard work on this! Looks good :) just a couple things

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Sebastian-ubs)


.github/workflows/storybook.yml line 1 at r1 (raw file):

name: Storybook

I think it would be hard to understand looking at the workflow name what this workflow is doing. Could we change this name and the file name to be a bit more descriptive? Like "Publish Storybook to GitHub Pages" and "publish-storybook.yml" or something?


.github/workflows/storybook.yml line 5 at r1 (raw file):

  push:
    branches:
      - Sebastian-ubs/issue411

Should this branch name be main?

@Sebastian-ubs
Copy link
Contributor Author

@tjcouch-sil seems it is using the user of the one that triggers the action by pushing. So the branch protection rule needs to be not too strict.
I can see the content being pushed here.
However it was not updating the github page there.

Copy link
Member

@tjcouch-sil tjcouch-sil left a comment

Choose a reason for hiding this comment

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

It seems mine doesn't work by running npx http-server storybook-static as indicated in this guide to testing the built storybook files locally, but it doesn't look like the same problem. After running npm run storybook:build locally, my
storybook-static/index.html is different than the one in the github-pages branch. My index.html looks nothing like what's pictured, which is github-pages/index.html:

image copy 1.png

I also have fewer files in my storybook-static than what github-pages has:

image.png

I don't know what's going on here, but there must be some environmental difference or something.

Regarding authentication, do you know if JamesIves/[email protected] provides a way to pass in a git auth token or something? I imagine there's probably a way to define secrets in the repo that we can pass into the workflow yml file to get special privileges to push to github-pages even if it's locked.

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Sebastian-ubs)

@Sebastian-ubs
Copy link
Contributor Author

Thank you. I recognized it also shows a blank page for me when built locally. Will need to have closer look into it.

Yes, JamesIves/[email protected] allowes to specify auth token secrets

Copy link
Member

@tjcouch-sil tjcouch-sil left a comment

Choose a reason for hiding this comment

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

I added branch protection on the github-pages branch. Would you mind making some kind of change that triggers it and seeing if it works please? We can figure out authentication if it doesn't work.

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Sebastian-ubs)

@Sebastian-ubs
Copy link
Contributor Author

@tjcouch-sil pushing with my credentials does not work, but we could add an auth token of an allowed automation-user

remote: error: GH006: Protected branch update failed for refs/heads/github-pages.
remote: error: You're not authorized to push to this branch. Visit https://docs.github.com/articles/about-protected-branches/ for more information.

@Sebastian-ubs
Copy link
Contributor Author

Thank you. I recognized it also shows a blank page for me when built locally. Will need to have closer look into it.

Probably it is because of // TODO: Make this work in production mode
https://github.com/paranext/paranext-core/blob/Sebastian-ubs/issue411/.storybook/main.ts#L35

Copy link
Member

@tjcouch-sil tjcouch-sil left a comment

Choose a reason for hiding this comment

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

I added a secret to the github-pages environment that will hopefully allow you to use PAGES_PAT to specify credentials in this workflow. Would you please try using it and let me know how it goes? I found this readme that shows how to use github actions secrets to provide a token (I know it's a different repo, but it looks like the syntax for specifying a token is the same).

Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @Sebastian-ubs)

Copy link
Member

@tjcouch-sil tjcouch-sil left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @Sebastian-ubs)


.github/workflows/storybook.yml line 25 at r4 (raw file):

          branch: github-pages
          folder: storybook-static
          GITHUB_TOKEN: ${{ secrets.PAGES_PAT }}

I believe this is supposed to be token: ${{ secrets.PAGES_PAT }}. I wonder if this has to do with this job not running. Otherwise I may have messed something up on my end.

Copy link
Member

@tjcouch-sil tjcouch-sil left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @Sebastian-ubs)


.github/workflows/storybook.yml line 25 at r4 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

I believe this is supposed to be token: ${{ secrets.PAGES_PAT }}. I wonder if this has to do with this job not running. Otherwise I may have messed something up on my end.

Hmm looks like it still didn't run. I changed something on my end. Would you mind pushing another fake change to see if the Storybook workflow runs? Thanks!

@Sebastian-ubs
Copy link
Contributor Author

there is goes. well yes you are right we actually told it to only run on a change in components or stories ;-)
Other thing: We switch looking into how to publish with Chromatic instead so was not paying much attention here.

Copy link
Member

@tjcouch-sil tjcouch-sil left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @Sebastian-ubs)


.github/workflows/storybook.yml line 25 at r4 (raw file):

Previously, tjcouch-sil (TJ Couch) wrote…

Hmm looks like it still didn't run. I changed something on my end. Would you mind pushing another fake change to see if the Storybook workflow runs? Thanks!

Looks like it's working again, but we changed multiple things at once. I'm going to change something on my end again. Would you mind making another fake commit please? Thanks!

Copy link
Member

@tjcouch-sil tjcouch-sil left a comment

Choose a reason for hiding this comment

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

Ah I didn't think about the file limitation. Ok gotcha. Do you want to continue this effort or drop it?

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @Sebastian-ubs)

@Sebastian-ubs
Copy link
Contributor Author

Not quite decided. Maybe let's pause looking into fixing it for now. I can make it a draft PR or anything if the helps anybody

Copy link
Member

@tjcouch-sil tjcouch-sil left a comment

Choose a reason for hiding this comment

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

Ok gotcha. Looks like the authentication method I provided didn't work :( we'll have to investigate further if we look into this again. Thanks for the hard work!

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @Sebastian-ubs)

@Sebastian-ubs Sebastian-ubs marked this pull request as draft September 7, 2023 16:22
@irahopkinson
Copy link
Contributor

@Sebastian-ubs can we close this PR since it hasn't moved for some time?

@Sebastian-ubs
Copy link
Contributor Author

Sebastian-ubs commented Dec 13, 2023

Yes, we will follow up in paranext/ux#3 based on priority

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