-
Notifications
You must be signed in to change notification settings - Fork 835
Charts: Organize Storybook controls with categories for better usability #45503
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: trunk
Are you sure you want to change the base?
Conversation
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
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.
Pull Request Overview
This PR organizes Storybook controls for chart components into logical categories to improve usability and navigation. Instead of having scattered controls, they are now grouped into meaningful sections like Data, Visual Style, Labels, Glyphs, Tooltips, and Dimensions.
Key changes include:
- Created a shared tooltip configuration module with categorized argTypes
- Added category groupings to existing chart controls across pie, line, and bar chart stories
- Organized dimension-related controls in the shared chart decorator
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
tooltip-config.tsx |
New module providing shared tooltip argTypes with Tooltips category |
index.ts |
Exports the new tooltip configuration |
chart-decorator.tsx |
Adds Dimensions category to shared chart controls |
pie-chart/stories/index.stories.tsx |
Categorizes pie chart controls into Visual Style, Labels, and Dimensions |
line-chart/stories/index.stories.tsx |
Adds Data, Dimensions, and Visual Style categories to line chart controls |
line-chart/stories/glyph.stories.tsx |
Groups glyph-related controls under Glyphs category |
bar-chart/stories/index.stories.tsx |
Organizes bar chart controls into Data and Visual Style categories |
changelog/charts-37-use-the-controls-to-put-similar-stories-together |
Documents the change |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
description: 'Grid line visibility', | ||
table: { category: 'Visual Style' }, | ||
}, | ||
seriesCount: { |
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 arg doesn't work in my testing. It doesn't affect how many series displayed.
description: 'Show glyphs in legend', | ||
table: { category: 'Glyphs' }, | ||
}, | ||
glyphType: { |
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 arg doesn't work in my testing. It doesn't affect how many series displayed.
description: 'Glyph shape', | ||
table: { category: 'Glyphs' }, | ||
}, | ||
glyphSize: { |
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 arg doesn't work in my testing. It doesn't affect how many series displayed.
title: 'JS Packages/Charts/Types/Line Chart', | ||
argTypes: { | ||
...lineChartMetaArgs.argTypes, | ||
seriesCount: { |
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 arg doesn't work in my testing. It doesn't affect how many series displayed.
export { legendArgTypes } from './legend-config'; | ||
|
||
// Tooltip configuration | ||
export { tooltipArgTypes, lineChartTooltipArgTypes } from './tooltip-config'; |
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 didn't find where those two exports are used 🤔
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.
Nice work on the groupings! Noted mostly the same non-functional controls as @kangzj
description: 'Number of data series', | ||
table: { category: 'Data' }, | ||
}, | ||
dimensionMode: { |
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 one has no effect for me either and isn't a LineChart prop, maybe we need some typing on these args (to help AI?)
fixes: #CHARTS-37
Proposed changes:
This PR organizes Storybook controls into logical categories (Data, Visual Style, Labels, Glyphs, Tooltips, Dimensions) to make the controls panel easier to navigate and use. All existing stories and exports are preserved - no consolidation or removal of stories.
Other information:
Does this pull request change what data or activity we track or use?
No, it does not.
Testing instructions: