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

chore: Fix links to Reactist components in storybook stories using LinkTo #795

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

engfragui
Copy link
Contributor

@engfragui engfragui commented Jul 31, 2023

Short description

Follow-up to #794 where I tried to fix a couple of broken links in our storybook stories.

While browsing a couple of Reactist storybook stories, I've noticed that there are some broken links that don't include the path "reactist":

CleanShot.2023-07-31.at.15.17.17.mp4

It turns out I didn't actually fix anything at all as the links are still missing the appropriate /reactist path which it seems not to be included in window.location.origin 😅

In this PR, I initially tried to address this by concatenating window.location.origin with window.location.pathname to (hopefully) get the correct Reactist URL path. However, that doesn't work either, because pathname will include /iframe.html as well (since the link is inside an iframe) so what I'm doing in this PR doesn't work either. To be fair, even if the concatenation had worked, it would not have looked great in our code 😅

I changed my approach to actually use the official Reactist add-on that "should" be used to link from a story to another story: https://storybook.js.org/addons/@storybook/addon-links (storybookjs/storybook#8618) and hopefully that will do the trick (for real, this time).

This seems to be working well when testing on the storybook dev version deployed via Chromatic (linked in the builds below)

CleanShot.2023-07-31.at.16.55.41.mp4

...so I have hopes it will work when it gets deployed to https://doist.github.io/reactist/

PR Checklist

  • Added tests for bugs / new features
  • Updated docs (storybooks, readme)
  • Executed npm run validate and made sure no errors / warnings were shown
  • Described changes in CHANGELOG.md
  • Bumped version in package.json and package-lock.json (npm --no-git-tag-version version <major|minor|patch>) ref
  • Reviewed and approved Chromatic visual regression tests in CI

Versioning

Bumped patch. This is just a doc change.

@engfragui engfragui force-pushed the francesca/fix-links-2 branch from cc666dc to 3efc882 Compare July 31, 2023 14:49
@engfragui engfragui changed the title chore: Fix links to Reactist components in storybook stories chore: Fix links to Reactist components in storybook stories using LinkTo Jul 31, 2023
@engfragui engfragui added the Show label Jul 31, 2023
@engfragui engfragui requested a review from gnapse July 31, 2023 14:58
@engfragui engfragui merged commit 46f7b1c into main Jul 31, 2023
@engfragui engfragui deleted the francesca/fix-links-2 branch July 31, 2023 14:58
@engfragui
Copy link
Contributor Author

I'm happy to confirm that this works as expected after being deployed to production 🎉

CleanShot.2023-07-31.at.17.13.24.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant