-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat(documentation): Set id/permalink for stories #2603
Conversation
|
Preview environment ready: https://preview-2603--swisspost-design-system-next.netlify.app |
Those code smells have nothing do it with this PR, fixed in #2605 |
# Conflicts: # packages/documentation/src/stories/components/button-group/button-group.stories.ts # packages/documentation/src/stories/components/forms/textarea/textarea.stories.ts
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 great! 👍
Can you just update the links to the foundation docs using the new id's?
Also, I'm not sure that using the titles as identifiers is the best solution because there will be discrepancies as soon as we update a component name... I would probably go for a unique id, maybe with an enum somewhere. What do you think?
Good catch!
I think this is a timeless question between slug vs id/uuid. It can be one or another, or mixed. I'm not really fond of only uuid as it's not user-friendly (and a bit prone to security issue). Using a mix doesn't make it more permanent except if we say that the slug is only here for human (e.g. /bzasdh/accessibility) and is ignored in the redirect process, but this is not how Storybook works, and it doesn't allow that. I think we just should care of any changes and redirect path using a middleware. So it's debatable, perhaps we should have survey in the roundtable meeting? |
Sure, let's talk about that with the team |
packages/documentation/src/stories/components/forms/custom-select/custom-select.stories.ts
Outdated
Show resolved
Hide resolved
packages/documentation/src/stories/components/forms/custom-select/custom-select.stories.ts
Outdated
Show resolved
Hide resolved
…ect/custom-select.stories.ts Co-authored-by: Alizé Debray <[email protected]>
…ect/custom-select.stories.ts Co-authored-by: Alizé Debray <[email protected]>
@@ -29,7 +30,7 @@ export default meta; | |||
// DECORATORS | |||
function externalControls(story: StoryFn, context: StoryContext) { | |||
const { args, canvasElement } = context; | |||
const togglerId = `${context.id}--button`; | |||
const togglerId = `button--${context.id}`; |
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 not valid otherwise, as we are targetting an id starting with a number later.
It was decided on the roundtable to used exclusively uuid. |
# Conflicts: # packages/documentation/src/stories/components/heading/heading.docs.mdx # packages/documentation/src/stories/foundation/typography/typography.docs.mdx # packages/documentation/src/stories/utilities/heading/heading.docs.mdx # packages/documentation/src/stories/utilities/heading/heading.stories.ts
Quality Gate passedIssues Measures |
Set an id for each story as in https://storybook.js.org/docs/configure/sidebar-and-urls#permalink-to-stories. This allows to separate the URL generation from the display name. The URL is generated with the id and the name of the story with the title.
You can change the title without changing the id so we can keep a kind of permalink.
Most of id are duplicated from title, except for foundations.