-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Feature] Text component: Update stories #593
Conversation
@@ -52,6 +56,9 @@ export const Body: Story = { | |||
const children = 'Lorem ipsum dolor sit amet.' | |||
|
|||
export const _Heading: Story = { | |||
argTypes: { | |||
tone: { control: 'radio', options: ['default', 'subtle', 'inverse'] }, |
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.
Be a slight improvement if the options could be pulled directly from HeaderProps.tone or BaseTones so if any are ever added/removed it doesn't break the 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.
TypeScript doesn't allow you to build an array from a type, but you can do the opposite. Declared and exported an array called baseToneValues
in Text.tsx
and updated the type to use that. The syntax is a little weird but solves the problem of hardcoding the options.
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.
Couple minor suggestions, but overall seems good to go.
Description of Change
Update Text component storybook stories:
tone
andsize
props when variant is'display'
'error'
tone option in Heading story.tone
to be conditionally hidden for the display variant, I could not add another condition to hide the error option for tone when the variant is heading. Because of this, the error tone is only hidden on the Heading story and will still be visible if switching variants on the Docs page or other stories.Also updates a missed requirement:
bottomSpacing
should only be fully customizable for Heading and Display variants. For Body, bottomSpacing should only be able to set to'none'
or left undefined to use the default.Testing Packages
N/A
Screenshots/Video
Screen.Recording.2024-11-18.at.11.07.45.AM.mov
Testing
PR Checklist
Code reviewer validation:
changelog
label applied if it's to be included in the changelogPublish
If changes warrant a new version per the versioning guidelines and the PR is approved and ready to merge:
main
into branchmain