improv: add custom modifier example #88

bernardobelchior · 2022-04-18T14:36:21Z

This PR adds an example of a custom Modifier. It creates a DodgeSizeModifier that changes an element's position and width depending on the number of elements in a band.

Custom Legend Example	Custom Modifier Example

bernardobelchior · 2022-04-21T09:47:02Z

lib/src/parse/parse.dart

@@ -431,32 +427,15 @@ void parse<D>(

    if (elementSpec.modifiers != null) {
      for (var modifier in elementSpec.modifiers!) {
-        GeomModifierOp geomModifier;
-        if (modifier is DodgeModifier) {


Most important change: we no longer need to check the instance type of modifier. Every modifier must implement a toGeomModifierOp that returns the corresponding GeomModifierOp, which allows everyone to specify their custom modifiers.

bernardobelchior · 2022-04-21T09:48:02Z

lib/src/geom/modifier/modifier.dart

+/// The only use of this class to pass parameters to [Modifier.toGeomModifierOp].
+/// A class is used, instead of named arguments, to avoid breaking changes when
+/// adding new fields to these parameters.
+class ToGeomModifierOpParams {


Decided to use a class here so we can add new parameters in the future without creating a breaking changes for consumers of this API.

Unfortunately Dart's named arguments require consumers to specify all parameters, if ones that aren't used, so adding a new argument to named arguments would be a breaking change.

bernardobelchior · 2022-04-21T09:49:07Z

lib/src/dataflow/tuple.dart

@@ -66,7 +66,7 @@ class Aes {
  final Label? label;

  /// The size of the tuple.
-  final double? size;
+  double? size;


Needed to remove the final keyword from this property so that it can be changed from a modifier.
This is similar to ggplot's dodge2 modifier that also changes the size of its elements

bernardobelchior · 2022-04-26T08:01:25Z

Hey, @entronad!
Could you let me know what you think of this PR? We're using graphic and this would be immensely helpful. We have highly dynamic data and would like to customize the chart depending on that data.

This changes core logic in graphic, but I think it does not break any consumers and adds the ability to customize graphic further, which I think is a win.
Please let me know if you have any concerns that you want me to address 😄

Thank you!

entronad · 2022-04-27T03:30:47Z

I would rather not merge this PR after a long time of thinking. Reasons are:

The dataflow, composed of operators are internal part and should not exposed in the graphic.dart. There are too complex concepts. Let alone to custom the operators.
I am not satisfied with the implementation of the modifier implementation operators now. They may be changed. So let's not to make it more complicated now.

But I really get some inspiration from this PR, thanks.

I think what you need (to auto adjust the bar size) can be implemented by a custom Shape, that is what recommended by Graphic. maybe that's not as simple as to change the modifier, but I think it worth a try.

bernardobelchior · 2022-04-27T08:11:32Z

Hey, @entronad!

I understand your concerns. On top of changing the width of the interval element, we have another use case where we need to stack interval elements, but the logic differs from the StackModifier.

In the StackModifier, negative values are summed up with positive values. In our use case, we want to separate them, so negative values only affect the stacking of negative values and positive values only affect the stacking of positive values.

Example:

In that use case, using a custom shape does not seem like a possible solution, right? What can we do there? That would be another use case for custom modifiers.

The dataflow, composed of operators are internal part and should not exposed in the graphic.dart. There are too complex concepts. Let alone to custom the operators.

I understand your concern. Maybe we can figure out a better API for this? Instead of having 3 different classes, we can probably have just one CustomModifier class that is subclassed?

I am not satisfied with the implementation of the modifier implementation operators now. They may be changed. So let's not to make it more complicated now.

Do you have any ideas in mind regarding how this might be implemented?

As you probably have noticed, I'd like to contribute and make graphic a better library, so if you have any suggestion, I could try implementing it, if it meant that we would have this done a bit faster 😄

Thanks!

entronad · 2022-04-27T09:54:30Z

Your cases resolved my doubt. I am convinced to add custom modifier now.
My idea about it now is:

an CustomModifer will be exposed to implement, you can override its modify method to do anything you want to change the AesGroups, that's the whole meaning of a modifer. You can also add your own properties for this sub class.
The trouble is: to modifier the position, there needs some internal arguments passed from the dataflow(like coord, form), while different modifers(stack or jitter) needs different internal arguments. That's why the implementation now is so verbose (involves three operators). I'm thinking to pass the max union set of these arguments to modifer, that may be concise.

I will think it over and make the commits. Maybe this PR will be used, but thank you all the same, tips from this PR are very usefull.

bernardobelchior · 2022-04-27T10:00:10Z

Good to know, @entronad! 😄
Let me know if I can help you with something! Thank you 😄

entronad · 2022-04-28T13:14:52Z

Added in v0.10.0
Users only need to custom the modify method of Modifier class.

bernardobelchior · 2022-04-28T14:10:24Z

Thank you, @entronad 🙏

Do you think it would make sense to update this PR to add the custom modifier example? Or should I close this PR?

bernardobelchior · 2022-04-28T14:12:58Z

Also, @entronad, it seems like AesGroups and ScaleConv isn't exported from graphic, so it isn't possible to override the modify method of a Modifier

entronad · 2022-04-28T15:18:01Z

Added these params types in v0.10.1

You are welcome to add these custom modifier examples, thank you very much. You can either edit this PR or post a new one.

bernardobelchior · 2022-04-28T15:47:25Z

lib/graphic.dart

+export 'src/scale/discrete.dart' show DiscreteScale, DiscreteScaleConv;
+export 'src/scale/continuous.dart' show ContinuousScale, ContinuousScaleConv;
+export 'src/scale/linear.dart' show LinearScale, LinearScaleConv;
+export 'src/scale/ordinal.dart' show OrdinalScale, OrdinalScaleConv;
+export 'src/scale/time.dart' show TimeScale, TimeScaleConv;


Need to add these exports for the modifier to work

bernardobelchior · 2022-04-28T15:52:59Z

example/lib/pages/polygon_custom.dart

+    final numGroups = aesGroups.length;
+    final groupHorizontalPadding = _kBaseGroupPaddingHorizontal / numGroups;
+    final invertedGroupPaddingHorizontal =
+        coord.invertDistance(groupHorizontalPadding, _kXAxis);


@entronad I need to get this coord, but I can't seem to get it with the current implementation. Do you have any idea how I can convert pixels into the [0, 1] range?

entronad · 2022-04-29T00:45:12Z

I will add all these.

entronad · 2022-04-30T16:53:49Z

I have add the coord in a new commit, please have a try.

bernardobelchior · 2022-05-02T08:39:16Z

lib/src/geom/modifier/modifier.dart

@@ -18,7 +18,7 @@ import 'package:graphic/src/shape/shape.dart';
 /// Rendering customization should be in the [Shape].
 abstract class Modifier extends CustomizableSpec {
  /// Modifies the position of element items.
-  /// 


Result of running flutter format ., which was failing the "ci" GitHub Action

bernardobelchior · 2022-05-02T08:39:27Z

It should be ready, @entronad 😄

bernardobelchior · 2022-05-02T08:41:25Z

Can we create a new release with the new exports? I'd like to use this modifier for another project 😄

entronad · 2022-05-02T13:34:51Z

Published in v0.10.2

bernardobelchior marked this pull request as ready for review April 18, 2022 14:37

bernardobelchior force-pushed the improv/custom-modifier branch 2 times, most recently from 991d9e6 to df03641 Compare April 21, 2022 09:46

bernardobelchior commented Apr 21, 2022

View reviewed changes

bernardobelchior force-pushed the improv/custom-modifier branch 3 times, most recently from ce76632 to 61bd9af Compare April 22, 2022 16:09

bernardobelchior force-pushed the improv/custom-modifier branch from 61bd9af to c9406f2 Compare April 28, 2022 15:46

bernardobelchior changed the title ~~improv: add support for custom modifiers~~ improv: add custom modifier example Apr 28, 2022

bernardobelchior commented Apr 28, 2022

View reviewed changes

bernardobelchior force-pushed the improv/custom-modifier branch from c9406f2 to accff22 Compare May 2, 2022 08:34

improv: add custom modifier example

ee7c943

bernardobelchior force-pushed the improv/custom-modifier branch from accff22 to ee7c943 Compare May 2, 2022 08:38

bernardobelchior commented May 2, 2022

View reviewed changes

entronad merged commit 939b259 into entronad:main May 2, 2022

bernardobelchior deleted the improv/custom-modifier branch May 2, 2022 13:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improv: add custom modifier example #88

improv: add custom modifier example #88

bernardobelchior commented Apr 18, 2022 •

edited

bernardobelchior Apr 21, 2022

bernardobelchior Apr 21, 2022

bernardobelchior Apr 21, 2022

bernardobelchior commented Apr 26, 2022

entronad commented Apr 27, 2022

bernardobelchior commented Apr 27, 2022 •

edited

entronad commented Apr 27, 2022

bernardobelchior commented Apr 27, 2022

entronad commented Apr 28, 2022

bernardobelchior commented Apr 28, 2022

bernardobelchior commented Apr 28, 2022

entronad commented Apr 28, 2022

bernardobelchior Apr 28, 2022

bernardobelchior Apr 28, 2022

entronad commented Apr 29, 2022

entronad commented Apr 30, 2022

bernardobelchior May 2, 2022

bernardobelchior commented May 2, 2022

bernardobelchior commented May 2, 2022

entronad commented May 2, 2022

improv: add custom modifier example #88

improv: add custom modifier example #88

Conversation

bernardobelchior commented Apr 18, 2022 • edited

bernardobelchior Apr 21, 2022

Choose a reason for hiding this comment

bernardobelchior Apr 21, 2022

Choose a reason for hiding this comment

bernardobelchior Apr 21, 2022

Choose a reason for hiding this comment

bernardobelchior commented Apr 26, 2022

entronad commented Apr 27, 2022

bernardobelchior commented Apr 27, 2022 • edited

entronad commented Apr 27, 2022

bernardobelchior commented Apr 27, 2022

entronad commented Apr 28, 2022

bernardobelchior commented Apr 28, 2022

bernardobelchior commented Apr 28, 2022

entronad commented Apr 28, 2022

bernardobelchior Apr 28, 2022

Choose a reason for hiding this comment

bernardobelchior Apr 28, 2022

Choose a reason for hiding this comment

entronad commented Apr 29, 2022

entronad commented Apr 30, 2022

bernardobelchior May 2, 2022

Choose a reason for hiding this comment

bernardobelchior commented May 2, 2022

bernardobelchior commented May 2, 2022

entronad commented May 2, 2022

bernardobelchior commented Apr 18, 2022 •

edited

bernardobelchior commented Apr 27, 2022 •

edited