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

Fix storybookUrl by removing iframe.html suffix #518

Merged
merged 2 commits into from
Feb 21, 2022

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Feb 11, 2022

update the action's storybookUrl output, to NOT include iframe.html as requested in #409

@linear
Copy link

linear bot commented Feb 11, 2022

@ndelangen ndelangen requested a review from ghengeveld February 11, 2022 10:06
@ndelangen ndelangen self-assigned this Feb 11, 2022
@ndelangen ndelangen added bug Classification: Something isn't working github action Classification: Relates to the GitHub Action labels Feb 11, 2022
@@ -107,7 +107,7 @@ async function runChromatic(options): Promise<Output> {
url: ctx.build?.webUrl,
code: ctx.exitCode,
buildUrl: ctx.build?.webUrl,
storybookUrl: ctx.build?.cachedUrl,
storybookUrl: ctx.build?.cachedUrl.replace(/iframe\.html.*$/, ''),
Copy link
Member

Choose a reason for hiding this comment

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

Why the .* at the end?

PS we also have a build.storybookUrl that we could request, perhaps could be more reliable?

Copy link
Member Author

Choose a reason for hiding this comment

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

I added that to also strip off any query params (which would not work for the manager)

Copy link
Member

Choose a reason for hiding this comment

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

There should never be any query params on cachedUrl I think, but can't hurt to guard against it I suppose.

@ndelangen ndelangen changed the title fix #409 by stripping ifram.html off (and everything after it) fix #409 by stripping iframe.html off (and everything after it) Feb 14, 2022
@ndelangen ndelangen requested a review from tmeasday February 14, 2022 14:46
@ndelangen
Copy link
Member Author

@ghengeveld WDYT?

Copy link
Contributor

@yannbf yannbf left a comment

Choose a reason for hiding this comment

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

LGTM! Also resharing Tom's question:

We also have a build.storybookUrl that we could request, perhaps could be more reliable than build.cachedUrl?

@ghengeveld ghengeveld changed the title fix #409 by stripping iframe.html off (and everything after it) Fix storybookUrl by removing iframe.html suffix Feb 21, 2022
@ghengeveld ghengeveld merged commit 96c23e1 into main Feb 21, 2022
@ghengeveld ghengeveld deleted the norbert/sb-104-github-action-storybookurl-output branch February 21, 2022 09:03
@ghengeveld
Copy link
Member

Released in 6.5.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Classification: Something isn't working github action Classification: Relates to the GitHub Action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants