Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Design system - PieChart stories #620

Merged
merged 5 commits into from
Jun 27, 2019
Merged

Conversation

bradfordjanice
Copy link
Contributor

This update to the PieChart component follows the Civic design system pattern established in #508, #553, and #565.

Working with the existing stories the following stories were created:

Standard - the common, default use of the PieChart component in the CIVIC platform
Custom - add knobs for all props of the PieChart component
Example: Simple - the simple story shows only the most basic props.

A notes file was added; it describes each story and list the props/knobs that can be changed.

Copy link
Contributor

@jaronheard jaronheard left a comment

Choose a reason for hiding this comment

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

The code looks great! Just some knobs to add and move around:

Standard story
Add "Data value" and "Data label" knobs to Data tab
Move "Use legend" from Labels to Design?

Custom story
Move "Data value" and "Data label" knobs from Custom to Data tab
Move "Use legend" from Labels to Design?
Adjust default width and height to below

        const chartHeight = number("Chart height", 350, {}, GROUP_IDS.CUSTOM);
        const chartWidth = number("Chart width", 650, {}, GROUP_IDS.CUSTOM);

@bradfordjanice
Copy link
Contributor Author

I made the requested changes. Adding "Data label" and "Data value" labels to the Standard story makes it consistent with the other chart component stories!

The reason I had not added it was because it wasn't used in the pie charts I found in the projects or in the existing PieChart story. The "Example: Simple" story shows the component using the default prop values for "Data label" and "Data value"; that seems the appropriate place.

After making these changes, the "Start angle" and "End angle" knobs don't work. It was working, and I can't find the problem. Looking at the component, it sets the value of startAngle and endAngle based on whether the prop halfDoughnut is true or false, not on the value of the knob. I'm not sure why it was working or why it isn't working after these changes... if you could take a look I'd appreciate it.

If I'm right that these knobs aren't currently changing the value of startAngle and endAngle, we could either delete them or update the component. By the way, none of the pie charts in the project have used startAngle and endAngle; perhaps it isn't useful.

@jaronheard
Copy link
Contributor

jaronheard commented Jun 26, 2019 via email

@bradfordjanice
Copy link
Contributor Author

In the last commit the "Start angle" and "End angle" knobs were removed from the Custom story. The Notes were updated with this change. The component was not changed.

@bradfordjanice bradfordjanice merged commit 82052a7 into master Jun 27, 2019
@bradfordjanice bradfordjanice deleted the design-system/PieChart branch June 27, 2019 16:53
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants