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

feat(radar-chart): Add minimum/maximum value overrides #1091

Closed
wants to merge 14 commits into from

Conversation

zmeggyesi
Copy link

@zmeggyesi zmeggyesi commented Jul 8, 2022

New values are not required, will fall back to previous functionality if omitted.

closes #1090, #1078

New values are not required, will fall back to previous functionality if omitted.

issue-id: imaNNeo#1090
@zmeggyesi
Copy link
Author

@imaNNeoFighT Pubspec intentionally left untouched, in case you want to step versions according to your own schedule.

I neglected the fact that points on the chart are scaled according to the max value in the dataset, not necessarily the provided the max value, which caused an offset in the drawn chart.

issue-id: imaNNeo#1090
@codecov
Copy link

codecov bot commented Jul 24, 2022

Codecov Report

Merging #1091 (7c3561b) into master (b5f469d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 7c3561b differs from pull request most recent head 1cd9f7e. Consider uploading reports for the commit 1cd9f7e to get more accurate results

@@           Coverage Diff           @@
##           master    #1091   +/-   ##
=======================================
  Coverage   86.19%   86.20%           
=======================================
  Files          45       45           
  Lines        2826     2828    +2     
=======================================
+ Hits         2436     2438    +2     
  Misses        390      390           
Impacted Files Coverage Δ
lib/src/chart/radar_chart/radar_chart_painter.dart 100.00% <100.00%> (ø)
...ib/src/chart/radar_chart/radar_chart_renderer.dart 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@imaNNeo
Copy link
Owner

imaNNeo commented Aug 3, 2022

BTW You need to write some unit-tests to pass the codecov pipeline

Make the new values non-nullable, and add some calculation logic based on imaNNeoFighT's suggestions

issue-id: imaNNeo#1090
@imaNNeo
Copy link
Owner

imaNNeo commented Aug 9, 2022

Something is wrong with your code style.
Please run make format and make analyze to see what's wrong.

@zmeggyesi
Copy link
Author

Sorry for the delay. I've cleaned up those orphaned variables and acted on the Analyzer warnings. I've also pulled the latest master, just to be on the safe side of things.

@@ -37,7 +37,9 @@ class RenderRadarChart extends RenderBaseChart<RadarTouchResponse> {
: _data = data,
_targetData = targetData,
_textScale = textScale,
super(targetData.radarTouchData, context);
super(targetData.radarTouchData, context) {
painter = RadarChartPainter();
Copy link
Owner

Choose a reason for hiding this comment

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

What is this painter?

@imaNNeo
Copy link
Owner

imaNNeo commented Jan 15, 2023

Any update?

@zmeggyesi
Copy link
Author

zmeggyesi commented Jan 17, 2023 via email

@imaNNeo imaNNeo closed this Nov 2, 2023
@eldrig0
Copy link

eldrig0 commented Jan 12, 2024

Is this feature implemented?

@imaNNeo
Copy link
Owner

imaNNeo commented Jan 22, 2024

We had a similar fix here: #1293

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.

Allow overriding of Radar Chart min/max values for tick calculation
4 participants