Core: Allow linking to kind/component ID #7648
Conversation
|
This pull request is being automatically deployed with ZEIT Now (learn more). 🔍 Inspect: https://zeit.co/storybook/monorepo/95v0zva8u |
lib/api/src/tests/stories.test.js
Outdated
| @@ -207,6 +207,25 @@ describe('stories API', () => { | |||
| expect(navigate).toHaveBeenCalledWith('/story/a--1'); | |||
| }); | |||
|
|
|||
| it( | |||
| 'navigates to the first child story of the parent story if a ' + | |||
There was a problem hiding this comment.
@lonyele what's up with this string concat?
There was a problem hiding this comment.
It's not a prettier error(80 character) but this string became too long and I ended up concatenating like this. Is there a better way? maybe template literal?
There was a problem hiding this comment.
I changed it to one line!
lib/api/src/modules/stories.ts
Outdated
| @@ -286,6 +286,11 @@ Did you create a path that uses the separator char accidentally, such as 'Vue <d | |||
| } else if (viewMode && firstLeaf) { | |||
| navigate(`/${viewMode}/${firstLeaf.id}`); | |||
| } | |||
| } else if (storiesHash[storyId] && storiesHash[storyId].children) { | |||
There was a problem hiding this comment.
@lonyele will this navigate multiple times if storyId is not immediately above the leaf?
There was a problem hiding this comment.
...if
storyIdnot immediately above the leaf?
Did you mean by when story is nested in multiple times? such as screenshot(Form -> Select -> validations) If that's the case, I think it is ok since the url always will be a working one when inside of if/else
const firstLeafId = storiesHash[storyId].children[0];
navigate(`/${viewMode}/${firstLeafId}`);If that's not the case, can you explain it more?
There was a problem hiding this comment.
storiesHash[storyId].children is not necessarily a leaf, is it? It's just a child node ID. What happens in the case that the child is not a leaf?
There was a problem hiding this comment.
Yeap, you are right. It was only covering the case when given storyId is one depth parent from the leaf story. I change the logic so that it can find leaf story from any depth
|
@shilman just a gentle ping... |
|
Thanks @lonyele! Will merge this into 5.3 after 5.2 is released later this week. 👍 |
…feature/existing-story-default-route
lib/api/src/modules/stories.ts
Outdated
| } else if (storiesHash[storyId]) { | ||
| // When story exists but if it is not the leaf story, it finds the proper | ||
| // leaf story from any depth. | ||
| const firstLeafStoryId = storiesHash[storyId].isLeaf |
There was a problem hiding this comment.
const firstLeafStoryId = findLeafStoryId(storiesHash, storyId);There was a problem hiding this comment.
@shilman oh wait, I'm writing a test now. BTW, this comment is on the previous file change. Please refresh for the new one
|
@shilman It's much better now. It will navigate to the leaf story from any depth. Consider the cases through this screenshot. Valid URL from the screenshot is
btw, I added 2 test cases for different depth. If test case naming or too many cases, please let me know how to fix it(I think it is little bit weird now) |
|
Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks! |
shilman
changed the title
Issue: #7582
What I did
I added the default route when story exists but child url is not specified
How to test