-
Notifications
You must be signed in to change notification settings - Fork 32
docs: add deep controls add on #3178
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
base: main
Are you sure you want to change the base?
Conversation
View your CI Pipeline Execution ↗ for commit fb3b96b
☁️ Nx Cloud last updated this comment at |
🚀 Styleguide deploy preview ready! |
📬Published Alpha Packages:@codecademy/[email protected] |
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
really
nice
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.
Looking good! 🔥🔥
I saw some things that could use some touch-ups, so left some thoughts/Qs.
options: ['normal', 'small', 'large'], | ||
}, | ||
control: 'radio', | ||
options: ['normal', 'small', 'large'], |
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.
The "CTAButton" story has size
control, which doesn't work, but the Figma seems to only have a single size. Looking into the CTAButton.tsx, it doesn't look like it uses sizeVariants
. TL;DR: should this size
prop be removed?
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.
on my list to clean up the button stories but gonna say this is out of scope
args: defaultProps, | ||
args: { | ||
title: 'Modal Modality', | ||
size: 'small', |
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.
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.
yeah i have a whole list of things id like to do within SB, id say this is out of scope for this work
}, | ||
}; | ||
|
||
export const LowEmphasisInfoTip: Story = { |
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.
export const Default: Story = { | ||
args: { | ||
children: 'This is an alert', | ||
cta: { children: 'Click Me', text: 'Hello' }, |
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 seems like it should be either children
or text
?
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.
Should this story include an onClose()
?
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.
yeah these props are weird
href: '#', | ||
onClick: () => { |
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.
Seems like href
and onClick
aren't be included in the controls at the same time :\ is it possible to add?
I'd imagine it's some typing thing happening between Anchor and Button
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.
its the onClick just doesnt want to show up so i gave up
control: { | ||
type: 'boolean', | ||
}, | ||
control: 'boolean', |
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.
Does this actually do anything?
I don't see it in the actual component lol
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.
i dont know, i was just cleaning up the controls so we dont accidentally do it wrong like we did half the time before
Overview
We've integrated the storybook-addon-deep-controls package into our Storybook setup to enhance the developer experience when working with complex, nested object props in our component stories. This addon transforms the way we interact with deeply nested properties by breaking them down into individual, user-friendly controls instead of requiring manual JSON editing.
How It Works
The addon automatically detects nested object properties in component args and creates individual controls for each primitive property within the nested structure. Instead of seeing a single JSON editor for complex objects, developers now see separate, typed controls for each nested property.
Key Technical Implementation:
Automatic Generation: The addon automatically splits any defined object args into multiple primitive controls for each deep primitive property passed into
args
. See Alert as an example.Dot Notation: If you need/want to customize the
argType
beyond what is inferred automatically, you can access the nested properties using dot notation inargTypes
, such as:'tipProps.placement'
for tooltip placement options'infotip.emphasis'
for infotip emphasis levels'popoverProps.beak'
for popover beak positioningTypeWithDeepControls<Meta<typeof Component>>
instead of the standardMeta<typeof Component>
to enable proper TypeScript support for deep controls.PR Checklist
Testing Instructions
PR Links and Envs