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.

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

Add capStyle for Pie Charts #1832

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add capStyle for Pie Charts #1832

wants to merge 3 commits into from

Conversation

bqubique
Copy link

@bqubique bqubique commented Jan 1, 2025

Enhances Pie Charts that have non-null & non-zero centerSpaceRadius. Fixes this issue: #1699
Based on this PR #1698

@@ -107,6 +118,7 @@ class PieChartData extends BaseChartData with EquatableMixin {
b.centerSpaceRadius,
t,
),
capStyle: capStyle,
Copy link
Owner

Choose a reason for hiding this comment

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

It should be capStyle: b.capStyle,

@@ -401,3 +414,9 @@ class PieChartDataTween extends Tween<PieChartData> {
@override
PieChartData lerp(double t) => begin!.lerp(begin!, end!, t);
}

enum CapStyle {
Copy link
Owner

Choose a reason for hiding this comment

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

Please write a documentation above it. And it seems it is specially for PieChart. So I think it makes sense to add Pie prefix for it.

@imaNNeo
Copy link
Owner

imaNNeo commented Jan 6, 2025

It feels weird in our PieChartSample 2.
We can use other animations for that. But it would be nice to see if there's any solution to handle different radiuses.

CleanShot.2025-01-06.at.21.21.58.mp4

@imaNNeo
Copy link
Owner

imaNNeo commented Jan 6, 2025

Also you need to update the unit-tests and add new ones for your feature.

@bqubique
Copy link
Author

bqubique commented Jan 6, 2025

@imaNNeo will do once I get the chance. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants