Implement clipData feature in scatter chart.

[prajwal27]

#897.

The Scatter chart can be used as a Bubble chart by providing different radii. So, when the radius is bigger and close to the axis of the chart, it is used to cross the axis and go out of the chart.

So, in my approach, I've added a new attribute in ScatterChartData called clipBubble. If we set it to true, it will clip the chart before drawing the spots such that the bubbles never cross the chart's axes. After the bubbles are drawn, it again restores the original region.

[imaNNeo]

@prajwal27 Thanks for contributing!

clipData property (defined in AxisChartData, which ScatterChartData extends from) is responsible to do exactly what you are looking for. I checked our scatter_chart_painter.dart; it seems we forgot to use it. It has no implementation logic at the moment.

We need to fix the functionality of clipData property to fix your issue.
Cheers!

[imaNNeo]

You can check line_chart_painter.dart:69 to get the idea.

[prajwal27]

So @imaNNeoFighT , shall I remove the new clipBubble attribute ?

[imaNNeo]

Yes, you're correct!
We need to use clipData to do that.

[prajwal27]

@imaNNeoFighT I checked the line_chart_painter.dart and I got this

 if (data.clipData.any) {
      canvasWrapper.saveLayer(
        Rect.fromLTWH(0, -40, canvasWrapper.size.width + 40,
            canvasWrapper.size.height + 40),
        Paint(),
      );

What's the logic of 40 here?

[imaNNeo]

To be honest I don't remember :))))
Try to remove it in the scatter chart and see does it work or not.

[prajwal27]

@imaNNeoFighT I've updated the PR as you've suggested. I did try and remove the 40. I didn't notice any change.
Let me know if this is good to merge.
PS: Our team is relying on this fix :) It'll be good if it can be merged asap even is the package gets upgraded later.

[codecov]

Codecov Report

Merging #898 (0a042d2) into dev (10867bf) will increase coverage by 0.18%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #898      +/-   ##
==========================================
+ Coverage   80.65%   80.84%   +0.18%     
==========================================
  Files          32       32              
  Lines        2973     3002      +29     
==========================================
+ Hits         2398     2427      +29     
  Misses        575      575              

Impacted Files	Coverage Δ
...src/chart/scatter_chart/scatter_chart_painter.dart	95.90% <100.00%> (+0.44%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 10867bf...0a042d2. Read the comment docs.

[imaNNeo]

Hi @prajwal27.
It's good to merge. Also, it would be great if you update the CHANGELOG.md file and add your fix.

[prajwal27]

Hi @prajwal27. It's good to merge. Also, it would be great if you update the CHANGELOG.md file and add your fix.

Yeah, is this considered as bug fix or enhancement?

[imaNNeo]

Hi @prajwal27. It's good to merge. Also, it would be great if you update the CHANGELOG.md file and add your fix.

Yeah, is this considered as bug fix or enhancement?

I think it is a bug fix (Because it was supposed to work).
Don't forget to mention #897 issue at the end of line (like other changelogs)

[prajwal27]

@imaNNeoFighT thanks for the quick suggestions and reviews. It would be great if you could merge this.

[imaNNeo]

Thank you @prajwal27
I'm trying my best to merge it ASAP.

There is a minor problem:

As you see, it doesn't work correctly with our borders (But we considered the border width in our code).
I checked it. It has a minor problem.

We should:
Add the left and top (borderWidth / 2)
Minus the right and top (borderWidth / 2)

I know you just copied it from our LineChart.
But the LineChart has exact same bug too, I'm going to fix it in another PR :)))

Below code is correct:

if (clip.left) {
  final borderWidth = border?.left.width ?? 0;
  left = getLeftOffsetDrawSize(holder) + (borderWidth / 2);
}
if (clip.top) {
  final borderWidth = border?.top.width ?? 0;
  top = getTopOffsetDrawSize(holder) + (borderWidth / 2);
}
if (clip.right) {
  final borderWidth = border?.right.width ?? 0;
  right = getLeftOffsetDrawSize(holder) +
      chartUsableSize.width -
      (borderWidth / 2);
}
if (clip.bottom) {
  final borderWidth = border?.bottom.width ?? 0;
  bottom = getTopOffsetDrawSize(holder) +
      chartUsableSize.height -
      (borderWidth / 2);
}

[prajwal27]

@imaNNeoFighT that's great. I didn't see it coming. Are we good to merge now?

[imaNNeo]

No please solve the problem I mentioned first, then we're going to merge it. (I said I will fix it in the LineChart)

[prajwal27]

Ohh my bad, sorry. I'll do it now.

[prajwal27]

@imaNNeoFighT modified the clip equation.

[imaNNeo]

Landed in dev.
Thanks for contributing!

[prajwal27]

It was fun understanding the code! You'll probably see me more often than not :)

[imaNNeo]

Thanks for being patient :)))
You know it's frustrating for some people.
But I need to make sure to merge a high-quality code.