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.

Sign up for GitHub

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
+122 −8
Draft

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3ed6120
feat(storybook): implement argTypeEnhancers for nice UX for Slot API …
Hotell Feb 13, 2025
302f495
feat(react-card-stories): use argTypeEnhancer for rendering
Hotell Feb 13, 2025
1d49538
fixup! feat(storybook): implement argTypeEnhancers for nice UX for Sl…
Hotell Feb 13, 2025
151ad36
Revert "feat(react-card-stories): use argTypeEnhancer for rendering"
Hotell Feb 13, 2025
2bc520b
fixup! fixup! feat(storybook): implement argTypeEnhancers for nice UX…
Hotell Feb 13, 2025
1f2d51f
feat: apply enhancer internallly within custom DocPage
Hotell Feb 13, 2025
1b764e4
refactor: dont mutate story object rather use scoped enhancer within …
Hotell Feb 14, 2025
c755e84
improve readability - move message before prop table
carlamntn Feb 21, 2025
5b1cb70
adjusted overrides to handle complex Slot types
carlamntn Feb 21, 2025
9accdbb
adjust for complex Slot types
carlamntn Feb 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions apps/public-docsite-v9/.storybook/preview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,42 @@ export const parameters = {
},
},
};

const slotRegex = /as\?:\s*"([^"]+)"/;

/**
*
* @type {import('@storybook/types').ArgTypesEnhancer}
*/
const withSlotEnhancer = context => {
Copy link
Contributor Author

@Hotell Hotell Feb 14, 2025 •
edited
Loading

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 updatedArgTypes = { ...context.argTypes };

Object.entries(updatedArgTypes).forEach(([key, argType]) => {
const type = argType.type;
if (!type || type.name !== 'other') {
return;
}

const value = type.value;

if (!value) {
return;
}

if (value.includes('WithSlotShorthandValue')) {
const match = value.match(slotRegex);
Copy link
Contributor Author

@Hotell Hotell Feb 19, 2025 •
edited
Loading

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`
  }
}

if (match) {
updatedArgTypes[key].table.type.summary = `Slot<\"${match[1]}\">`;
} else {
updatedArgTypes[key].table.type.summary = `Slot`;
}
}
});

return updatedArgTypes;
};

/**
* @type {import('@storybook/types').ArgTypesEnhancer[]}
*/
export const argTypesEnhancers = [withSlotEnhancer];
91 changes: 83 additions & 8 deletions apps/public-docsite-v9/src/DocsComponents/FluentDocsPage.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
Stories,
type DocsContextProps,
} from '@storybook/addon-docs';
import type { PreparedStory, Renderer } from '@storybook/types';
import type { PreparedStory, Renderer, StrictArgTypes } from '@storybook/types';
import type { SBEnumType } from '@storybook/csf';
import { makeStyles, shorthands, tokens, Link, Text } from '@fluentui/react-components';
import { InfoFilled } from '@fluentui/react-icons';
Expand Down Expand Up @@ -71,6 +71,34 @@ const useStyles = makeStyles({
flexDirection: 'column',
gap: tokens.spacingVerticalXS,
},
slotAPIs: {
display: 'flex',
gap: tokens.spacingHorizontalM,

border: `1px solid ${tokens.colorNeutralStroke1}`,
borderRadius: tokens.borderRadiusMedium,
padding: tokens.spacingHorizontalM,
margin: `0 ${tokens.spacingHorizontalM}`,
},
slotAPIsInfo: {
display: 'flex',
gap: tokens.spacingHorizontalM,

border: `1px solid ${tokens.colorNeutralStroke1}`,
borderRadius: tokens.borderRadiusMedium,
padding: tokens.spacingHorizontalM,
margin: `0 ${tokens.spacingHorizontalM}`,
},
slotAPIsMessage: {
display: 'flex',
flexDirection: 'column',
gap: tokens.spacingVerticalXS,
},
slotAPIsIcon: {
alignSelf: 'center',
color: tokens.colorBrandForeground1,
fontSize: '24px',
},
});

const useVideoClasses = makeStyles({
Expand Down Expand Up @@ -133,26 +161,72 @@ 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


function withSlotEnhancer(story: PreparedStory) {
type InternalComponentApi = {
__docgenInfo: { props?: Record<string, { type: { name: string } }> };
[k: string]: unknown;
};
const component = story.component as InternalComponentApi;
const docGenProps = component?.__docgenInfo?.props;

if (!docGenProps) {
return component;
}

Object.entries(docGenProps).forEach(([key, argType]) => {
const value: string = argType?.type?.name;
if (value.includes('WithSlotShorthandValue')) {
const match = value.match(slotRegex);
if (match) {
component.__docgenInfo.props![key].type.name = `Slot<\"${match[1]}\">`;
// @ts-expect-error - storybook doesn't ship proper types (value is missing)
updatedArgTypes[key].type.value = `Slot<\"${match[1]}\">`;
} else {
component.__docgenInfo.props![key].type.name = `Slot`;
// @ts-expect-error - storybook doesn't ship proper types (value is missing)
updatedArgTypes[key].type.value = `Slot`;
}
}
});

return component;
}

const RenderArgsTable = ({
hideArgsTable,
story,
argTypes,
}: {
story: PrimaryStory;
hideArgsTable: boolean;
argTypes: StrictArgTypes;
}) => {
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

// const hasSlot = Object.keys(story.argTypes).some(key =>
// story.argTypes[key].table?.type?.summary?.startsWith('Slot<'),
// );
return hideArgsTable ? null : (
<>
<ArgsTable of={primaryStory.component} />
{primaryStory.argTypes.as && primaryStory.argTypes.as?.type?.name === 'enum' && (
{story.argTypes.as && story.argTypes.as?.type?.name === 'enum' && (
<div className={styles.nativeProps}>
<InfoFilled className={styles.nativePropsIcon} />
<div className={styles.nativePropsMessage}>
<b>
Native props are supported <span role="presentation">🙌</span>
</b>
<span>
All HTML attributes native to the {getNativeElementsList(primaryStory.argTypes.as.type.value)}, including
all <code>aria-*</code> and <code>data-*</code> attributes, can be applied as native props on this
component.
All HTML attributes native to the {getNativeElementsList(story.argTypes.as.type.value)}, including all{' '}
<code>aria-*</code> and <code>data-*</code> attributes, can be applied as native props on this component.
</span>
</div>
</div>
)}

<ArgsTable of={component} />
</>
);
};
Expand All @@ -179,6 +253,7 @@ const RenderPrimaryStory = ({
export const FluentDocsPage = () => {
const context = React.useContext(DocsContext);
const stories = context.componentStories();

const primaryStory = stories[0];
const primaryStoryContext = context.getStoryContext(primaryStory);

Expand Down Expand Up @@ -219,7 +294,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

<Stories />
</div>
<div className={styles.toc}>
Expand Down
Loading