Doc Blocks: Add support for of prop to Primary block
#23849
Conversation
Wilson2k
requested review from
JReinhold,
cdedreuille,
kylegach and
jonniebigodes
as code owners
|
Hi everyone, this is my first contribution to the codebase. Please let me know if I can improve anything, thanks! |
|
|
||
| const meta = { | ||
| const meta: Meta<typeof Primary> = { |
There was a problem hiding this comment.
Curious why you changed from the satisfies operator to direct assignment of the type?
There was a problem hiding this comment.
Hi, thanks for reviewing my code! I changed it based off other PRs and off of Description.stories.tsx. But this is a good question, and after some research, I'll change it back to satisfies since it's just better for validation and type safety.
docs/api/doc-block-primary.md
Outdated
|
|
||
| Type: CSF file exports | ||
|
|
||
| Specifies the primary (first) story to be rendered. |
There was a problem hiding this comment.
Can we state something similar to Meta's of prop description?
| Specifies the primary (first) story to be rendered. | |
| Specifies which CSF file is used to find the first story, which is then rendered by this block. Pass the full set of exports from the CSF file (not the default export!). |
There was a problem hiding this comment.
Yeah sure thing, I'll push a commit with the other fix today.
…son2k/storybook into update-primary-block-of-prop
There was a problem hiding this comment.
@Wilson2k, thanks for addressing the feedback so promptly. Appreciate it 🙏 ! Documentation-wise, it's all good on my end. I'll defer to the other maintainers to see if they have anything else to say about this great contribution.
cc @JReinhold
Hope you have a great day.
Stay safe
|
Hey @JReinhold could you please take a look? |
MIGRATION.md
Outdated
| ##### Primary block | ||
|
|
||
| The `Primary` block now also accepts an `of` prop as described above. It still accepts being passed `name` or no props at all. | ||
|
|
There was a problem hiding this comment.
These migration guides are between specific versions, adding this note here communicates that this was introduced in 7.0, while it's probably being introduced in 7.6.
I suggest you create a new top section below the Table Of Content called "From version 7.5.0 to 7.6.0", that includes this content. Given that this will move it "out" of the doc block section, you probably need to add a bit more context and link to this doc block section.
There was a problem hiding this comment.
Ok sounds good, I'll push a commit along with the other one updating the docs.
Specify 'meta' as the only valid type for useOf Co-authored-by: Jeppe Reinhold <jeppe@reinhold.is>
| const resolvedOf = useOf(of || 'meta', ['meta']); | ||
| story = getPrimaryFromResolvedOf(resolvedOf); |
There was a problem hiding this comment.
I'm sorry for not realising this sooner, I just thought of it now. I think now that we supply 'meta' as the only valid type, we don't need the "complex" error handling in getPrimaryFromResolvedOf, as that is already done within useOf based on the second argument - it should even be typed correctly.
So maybe you could try this and remove getPrimaryFromResolvedOf entirely and see if that still works?
| const resolvedOf = useOf(of || 'meta', ['meta']); | |
| story = getPrimaryFromResolvedOf(resolvedOf); | |
| const resolvedOf = useOf(of || 'meta', ['meta']); | |
| story = resolvedOf.csfFile.stories[0] || null; |
There was a problem hiding this comment.
Hey, sorry for the late reply, I just tested it and it seems to work fine. Good catch, I'll push a commit with the changes.
JReinhold
changed the title
of prop to Primary block
Partially implements #22490
What I did
ofprop via useOf with "meta" as only valid typeofprop toSubtitle#22552 as an exampleHow to test
yarn storybook:blocksChecklist
MIGRATION.MD
Maintainers
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli/src/sandbox-templates.ts["cleanup", "BREAKING CHANGE", "feature request", "bug", "build", "documentation", "maintenance", "dependencies", "other"]