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

feat(storybook): implement argTypeEnhancers for improved Slot API docs rendering #33838

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Hotell
Copy link
Contributor

@Hotell Hotell commented Feb 13, 2025

Previous Behavior

image

New Behavior

image

Leverages Storybook Arg enhancer hook to tweak ouput as needed without any changes necessary in v9 codebase ( not affecting consumers of v9 )

Related Issue(s)

@Hotell Hotell changed the title Sb/implement arg type enhancers for slot feat(storybook): implement argTypeEnhancers for nice UX for Slot API rendering Feb 13, 2025
Copy link

github-actions bot commented Feb 13, 2025

📊 Bundle size report

✅ No changes found

@fabricteam
Copy link
Collaborator

🕵 FluentUIV0 No visual regressions between this PR and main

Copy link

Pull request demo site: URL

@Hotell Hotell changed the title feat(storybook): implement argTypeEnhancers for nice UX for Slot API rendering feat(storybook): implement argTypeEnhancers for improved Slot API docs rendering Feb 13, 2025
*
* @type {import('@storybook/types').ArgTypesEnhancer}
*/
const withSlotEnhancer = context => {
Copy link
Contributor Author

@Hotell Hotell Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this works as expected with default DocsPage as it updates context properly which is then obtained and used for rendering the view.

https://github.com/storybookjs/storybook/blob/release-7-6/code/ui/blocks/src/blocks/ArgsTable.tsx#L220

because we override DocsPage by our custom implementation, this is no longer working unfortunately, as the context won't propagate as expected within default SB ArgsTable component.

Task:

  • determine if this can be set properly so SB ArgsTable would render enhanced data - if doable we don't have to execute the override inline within the custom DocsPage implementation

const styles = useStyles();

const { component } = withSlotEnhancer(story);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Task:

  • verify that this doesn't mutate story object as we don't wanna do that to prevent any unwanted side effects

@@ -219,7 +256,7 @@ export const FluentDocsPage = () => {
{videos && <VideoPreviews videos={videos} />}
</div>
<RenderPrimaryStory primaryStory={primaryStory} skipPrimaryStory={skipPrimaryStory} />
<RenderArgsTable primaryStory={primaryStory} hideArgsTable={hideArgsTable} />
<RenderArgsTable story={primaryStory} hideArgsTable={hideArgsTable} argTypes={primaryStoryContext.argTypes} />
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these argTypes are properly transformed via preview enhance in preview.js although we cannot use them it seems

Task:

  • revert this argTypes prop change

@Hotell
Copy link
Contributor Author

Hotell commented Feb 14, 2025

@carlamntn please see comments and Task to follow-up/finish this PR

ty

@@ -133,22 +133,58 @@ const getNativeElementsList = (elements: SBEnumType['value']): JSX.Element => {
);
};

const RenderArgsTable = ({ hideArgsTable, primaryStory }: { primaryStory: PrimaryStory; hideArgsTable: boolean }) => {
const slotRegex = /as\?:\s*"([^"]+)"/;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems this is not catching all Slot apis as expected for example (Avatar Component / https://react.fluentui.dev/?path=/docs/components-avatar--docs):

image

Todo:

  • make sure we are catching all actual Slot definitions


// we are interested only on raw strings ( which is case of non Storybook supported types)
if (typeof value === 'string') {
const match = value.match(slotRegex);
Copy link
Contributor Author

@Hotell Hotell Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to tweak the overrides if the Slot type is complex ( eg prop is using Slot<typeof SomeOtherComponent>:

if( value.includes('WithSlotShorthandValue')){
  const match = value.match(slotRegex);
  if(match) {
    // we know that the origin prop was defined via `Slot<'html elemen'>`
   // update value to `Slot<\"${match[1]}\">`
  } else {
  // we know that the origin prop is using non trivial type structure like `Slot<typeof PresenceBadgeProps>` -> we just fallback to `Slot` because we are unable to infer the correct value
// update to `Slot`
  }
}

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

Successfully merging this pull request may close these issues.

[docs]: improve Storybook prop table for Slot<T> type
3 participants