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 isMinValueAtCenter property to RadarChartData #1682

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

Conversation

soraef
Copy link
Contributor

@soraef soraef commented Jun 14, 2024

Summary

This PR adds the isMinValueAtCenter flag to the RadarChartPainter class to allow the minimum value to be centered in the RadarChart.

Related Issues

Changes

  • Added isMinValueAtCenter flag to RadarChartData
  • Updated RadarChartPainter methods to support centering the minimum value when isMinValueAtCenter is true

isMinValueAtCenter Comparison

isMinValueAtCenter is false isMinValueAtCenter is true
isMinValueAtCenter is false isMinValueAtCenter is true

@soraef
Copy link
Contributor Author

soraef commented Jun 14, 2024

Thank you for the wonderful product. I use it all the time.

After implementing the changes, I realized that the isMinValueAtCenter flag might not be necessary. Should the minimum value be centered by default? If the current behavior is a bug, it would be better to make the minimum value centered by default instead of adding a flag. This would simplify the code and avoid unnecessary complexity.

@imaNNeo
Copy link
Owner

imaNNeo commented Jun 20, 2024

Yes I think it is a bug and it should always be centered

@soraef
Copy link
Contributor Author

soraef commented Oct 16, 2024

I identified a bug where the behavior did not center around zero as expected, so I made corrections. However, I couldn’t get the test to pass. I'm not sure how to adjust the test code to reflect the fix, so I've left it unresolved.

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