Add shadow for Pie Chart #1497

samir7osny · 2023-11-21T21:45:41Z

This adds the drop shadow to the Pie Chart with some cool customizations

You can add simply a drop shadow like this (check the pie chart sample 4 here)
Also you can simulate the third dimension like this (check the pie chart sample 5 here)

codecov · 2023-11-24T18:14:30Z

Codecov Report

Attention: 52 lines in your changes are missing coverage. Please review.

Comparison is base (b3529d3) 86.67% compared to head (58a65cf) 85.34%.
Report is 26 commits behind head on master.

Files	Patch %	Lines
lib/src/chart/pie_chart/pie_chart_painter.dart	9.25%	49 Missing ⚠️
lib/src/extensions/bar_chart_data_extension.dart	81.25%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1497      +/-   ##
==========================================
- Coverage   86.67%   85.34%   -1.33%     
==========================================
  Files          45       45              
  Lines        3016     3078      +62     
==========================================
+ Hits         2614     2627      +13     
- Misses        402      451      +49

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

imaNNeo · 2023-11-24T19:45:00Z

It seems there is an issue, it freezes the app (tested on iPhone and macOS)

samir7osny · 2023-11-25T01:19:05Z

It seems there is an issue, it freezes the app (tested on iPhone and macOS)

Can you test again? I changed the number of layers to be drawn, it should fix this issue.

3DExtended · 2023-11-25T19:06:21Z

Tested on iPhone 15 Pro and simulator on MacBook Pro (2023). Seems to work now!

imaNNeo · 2023-12-01T00:32:00Z

Okay, I found the freeze issue, if you change the offset to Offset(0, 0), it freezes the app process (it seems a low-level exception happens)

imaNNeo · 2023-12-01T00:42:44Z

lib/src/chart/base/base_chart/base_chart_data.dart

+        offset = offset ?? const Offset(3, 3),
+        color = color ?? Colors.black.withOpacity(0.5),
+        numberOfLayers = numberOfLayers ?? 1,
+        minDarkenValue = minDarkenValue ?? 0,


Please define a better value for minDarkenValue and maxDarkenValue to simplify the usage. (for example 40 and 70 as you used in examples)

imaNNeo · 2023-12-01T00:43:52Z

lib/src/chart/base/base_chart/base_chart_data.dart

+  final bool show;
+  final Offset offset;
+  final Color color;
+  final MaskFilter? maskFilter;


Do we need it for a simple shadow? If yes, please make it non-nullable and define a default value to make it simple for user/developer to implement a shadow

imaNNeo · 2023-12-01T00:44:49Z

lib/src/chart/base/base_chart/base_chart_data.dart

@@ -78,6 +78,79 @@ class FlBorderData with EquatableMixin {
      ];
 }

+/// Holds data to drawing shdow for the chart.


Typo for shdow

imaNNeo · 2023-12-01T00:45:24Z

lib/src/chart/base/base_chart/base_chart_data.dart

@@ -78,6 +78,79 @@ class FlBorderData with EquatableMixin {
      ];
 }

+/// Holds data to drawing shdow for the chart.
+class FlShadowData with EquatableMixin {
+  /// [show] Determines showing or hiding border around the chart.


hiding border is wrong. It should be hiding shadow I think

imaNNeo · 2023-12-01T00:45:47Z

lib/src/chart/base/base_chart/base_chart_data.dart

+              maxDarkenValue == null ||
+              maxDarkenValue >= minDarkenValue,
+        );
+  final bool show;


Please write the code doc for the properties.

imaNNeo · 2023-12-01T00:46:04Z

lib/src/chart/base/base_chart/base_chart_data.dart

+
+  bool isVisible() => show;
+
+  /// Copies current [FlBorderData] to a new [FlBorderData],


FlBorderData is wrong here

imaNNeo · 2023-12-01T00:46:26Z

lib/src/chart/pie_chart/pie_chart_data.dart

@@ -213,6 +214,8 @@ class PieChartSectionData {
  /// 1.0 means near the outside of the [PieChart].
  final double badgePositionPercentageOffset;

+  final FlShadowData? shadow;


Write the code doc please

imaNNeo · 2023-12-01T00:48:11Z

lib/src/chart/pie_chart/pie_chart_data.dart

@@ -268,6 +273,7 @@ class PieChartSectionData {
        b.badgePositionPercentageOffset,
        t,
      ),
+      shadow: b.shadow,


Can we have a FlShadowData.lerp() here? It makes it good when animating between states.

imaNNeo · 2023-12-01T00:50:17Z

Please update the pie_chart.md and base_chart.md docs.
Also please write some unit-tests. Read more about the contributing guideline here

imaNNeo · 2023-12-01T00:52:36Z

lib/src/chart/pie_chart/pie_chart_painter.dart

+        break;
+      }
+
+      final sectionPath = generateSectionPath(


Please store this variable in the outer scope to make it possible to re-use it on line 168 when we want to draw actual paths to improve the performance.

imaNNeo · 2023-12-01T00:53:49Z

lib/src/chart/base/base_chart/base_chart_data.dart

+      offset: offset ?? this.offset,
+      color: color ?? this.color,
+      maskFilter: maskFilter ?? this.maskFilter,
+      numberOfLayers: numberOfLayers ?? 1,


You need to use this.numberOfLayers instead of hard-coded 1

imaNNeo · 2023-12-01T00:54:01Z

lib/src/chart/base/base_chart/base_chart_data.dart

+      color: color ?? this.color,
+      maskFilter: maskFilter ?? this.maskFilter,
+      numberOfLayers: numberOfLayers ?? 1,
+      minDarkenValue: minDarkenValue ?? 0,


You need to use this.minDarkenValue instead of hard-coded 0

imaNNeo · 2023-12-01T00:54:11Z

lib/src/chart/base/base_chart/base_chart_data.dart

+      maskFilter: maskFilter ?? this.maskFilter,
+      numberOfLayers: numberOfLayers ?? 1,
+      minDarkenValue: minDarkenValue ?? 0,
+      maxDarkenValue: maxDarkenValue ?? 0,


You need to use this.maxDarkenValue instead of hard-coded 0

imaNNeo · 2023-12-01T00:58:38Z

Also, please use different style/color/value for the new charts. You just copy-pasted the sample1 and they look exactly the same except the shadow. It doesn't feel good to see some duplicate charts.
You can combine the sample 4 and 5 together and add some options to change the shadow style (it helps users/developers to understand how it works).
For example you can add a slider to allow user/developer to increase the shadow layers, offset, ...
Or when you hover/touch it, instead of adding stroke radius, you can increase/decrease the shadow.

Samir Hosny Mohamed added 4 commits November 21, 2023 22:40

add shadow to pie chart

d7508c7

use shift instead of transform

d4999e4

format

895cb4b

Merge branch 'master' into feature/add-shadow-for-piechart

58ae152

samir7osny mentioned this pull request Nov 21, 2023

PieChart backdrop shadow #1482

Open

replace the sample 4 with sample 5

2845951

fix the example numberOfLayers

58a65cf

imaNNeo reviewed Dec 1, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add shadow for Pie Chart #1497

Add shadow for Pie Chart #1497

samir7osny commented Nov 21, 2023 •

edited

codecov bot commented Nov 24, 2023 •

edited

imaNNeo commented Nov 24, 2023

samir7osny commented Nov 25, 2023

3DExtended commented Nov 25, 2023

imaNNeo commented Dec 1, 2023

imaNNeo Dec 1, 2023

imaNNeo Dec 1, 2023

imaNNeo Dec 1, 2023

imaNNeo Dec 1, 2023

imaNNeo Dec 1, 2023

imaNNeo Dec 1, 2023

imaNNeo Dec 1, 2023

imaNNeo Dec 1, 2023

imaNNeo commented Dec 1, 2023

imaNNeo Dec 1, 2023

imaNNeo Dec 1, 2023

imaNNeo Dec 1, 2023 •

edited

imaNNeo Dec 1, 2023 •

edited

imaNNeo commented Dec 1, 2023


		bool isVisible() => show;

		/// Copies current [FlBorderData] to a new [FlBorderData],

Add shadow for Pie Chart #1497

Are you sure you want to change the base?

Add shadow for Pie Chart #1497

Conversation

samir7osny commented Nov 21, 2023 • edited

codecov bot commented Nov 24, 2023 • edited

Codecov Report

imaNNeo commented Nov 24, 2023

samir7osny commented Nov 25, 2023

3DExtended commented Nov 25, 2023

imaNNeo commented Dec 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imaNNeo commented Dec 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imaNNeo Dec 1, 2023 • edited

Choose a reason for hiding this comment

imaNNeo Dec 1, 2023 • edited

Choose a reason for hiding this comment

imaNNeo commented Dec 1, 2023

samir7osny commented Nov 21, 2023 •

edited

codecov bot commented Nov 24, 2023 •

edited

imaNNeo Dec 1, 2023 •

edited

imaNNeo Dec 1, 2023 •

edited