-
-
Notifications
You must be signed in to change notification settings - Fork 238
feat: add story viewer #1326
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
base: main
Are you sure you want to change the base?
feat: add story viewer #1326
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
JReinhold
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is cool!
I definitely like the in-npmx navigator better, it's just better DX.
However I'm thorn between it being in the top vs on the side. Putting it in the top (next to "code") would make the navigation experience consistent, it does something very similar to the code view, so putting them together makes sense from a user-expectation standpoint.
However it also works similar to a playground, so having it in the side with the other playground links makes sense too. But if we build it as the in-npmx experience (which IMO we should), it would act differently from all the other playground entries, which AFAIK are all external links with target="_blank".
WDYT @danielroe?
| query: { storyid: node.storyId }, | ||
| } | ||
| } | ||
| // For directories - navigate to first story in that directory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think instead we want to follow the existing navigation pattern in the Code view, where clicking a directory only expands it (and changes it icon) but doesn't select any stories - only navigating to a story actually selects it.
I know that Storybook itself does the latter, but here I think we want to be consistent with npmx instead.
| return getFileIcon(node.name) | ||
| } | ||
|
|
||
| const { toggleDir, isExpanded, autoExpandAncestors } = useStoryTreeState(props.baseUrl) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think autoExpandAncestors is the right idea, but it's not active at the moment.
| <span v-if="currentStory.story?.tags" class="text-fg-subtle"> | ||
| {{ currentStory.story.tags.join(', ') }} | ||
| </span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think tags is valuable information here, in 99 % of the cases it's internal information.
|
|
||
| const playgroundLinks = computed(() => [ | ||
| ...readmeData.value.playgroundLinks, | ||
| ...(packageJson.value?.storybook |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
packages can have a storybook property without having a storybook.url property, so we need to check for that here instead. Storybook addons have metadata in the same storybook property, but with no url, eg. https://npmxdev-git-fork-sacrosanctic-story-viewer-npmx.vercel.app/package-code/@storybook/addon-docs/v/10.2.8/package.json#L118-L124
closes #1298
I added both an integrated viewer and an external link just to see how it would feel.
The UI is really rough right now, and had a lot of assist from LLM, so I'll need to clean it up.