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

Added Tooltip border color and width support #917

Closed
wants to merge 45 commits into from

Conversation

fpplm
Copy link

@fpplm fpplm commented Feb 23, 2022

vincenzopalazzo and others added 30 commits January 16, 2022 23:37
In other to improve the test coverage, we should remove the example directory
because we don't want that also the example enter inside the scoring.

Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
…ms on showing titles when min, max values are below than 1.0)
* feat: Introduce 'clipBubble' attribute to scatter chart.

* fix: remove `clipBubble` attribute and implement `clipData` function in scatter chart.

* added issue fix imaNNeo#897 to changelog.

* modify clip equation to resolve border issue.

Co-authored-by: prajwal27wiijii <prajwal@wiijii.co>
* Fix lineChart clipData issue with border, imaNNeo#898

* Update CHANGELOG.md
## 0.0.1 - Released on (2019 June 4)
Copy link
Owner

Choose a reason for hiding this comment

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

Why this line is changed?

@@ -642,6 +642,12 @@ class BarTouchTooltipData with EquatableMixin {
/// The tooltip background color.
final Color tooltipBgColor;

/// The tooltip border color.
final Color tooltipBorderColor;
Copy link
Owner

Choose a reason for hiding this comment

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

I think it's better to have a BorderSide (which contains color and width) Instead of defining two variables.

Copy link
Author

Choose a reason for hiding this comment

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

good idea

@@ -668,6 +672,10 @@ class BarChartPainter extends AxisChartPainter<BarChartData> {
bottomLeft: radius,
bottomRight: radius);
_bgTouchTooltipPaint.color = tooltipData.tooltipBgColor;
if (tooltipData.tooltipBorderWidth > 0) {
Copy link
Owner

Choose a reason for hiding this comment

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

Can you please check the opacity of color too here?
Because sometimes people set a transparent color, which we don't need to draw anything.
(Maybe it's better to have an extension function like borderSide.isVisible())

extension BorderSideExtension on BorderSide {
  bool isVisible() => color.opacity > 0 && width > 0;
}

Copy link
Owner

Choose a reason for hiding this comment

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

Please apply it to other places.

@codecov
Copy link

codecov bot commented Feb 28, 2022

Codecov Report

Merging #917 (f566516) into dev (94e186c) will decrease coverage by 0.14%.
The diff coverage is 62.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #917      +/-   ##
==========================================
- Coverage   80.99%   80.85%   -0.15%     
==========================================
  Files          32       32              
  Lines        3026     3050      +24     
==========================================
+ Hits         2451     2466      +15     
- Misses        575      584       +9     
Impacted Files Coverage Δ
lib/src/chart/bar_chart/bar_chart_painter.dart 86.17% <62.50%> (-0.39%) ⬇️
lib/src/chart/line_chart/line_chart_painter.dart 80.15% <62.50%> (-0.19%) ⬇️
...src/chart/scatter_chart/scatter_chart_painter.dart 95.08% <62.50%> (-0.88%) ⬇️

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 94e186c...f566516. Read the comment docs.

@imaNNeo imaNNeo force-pushed the dev branch 2 times, most recently from 37fb6f1 to 711c937 Compare March 3, 2022 20:46
@imaNNeo imaNNeo deleted the branch imaNNeo:dev March 11, 2022 19:57
@imaNNeo imaNNeo closed this Mar 11, 2022
@imaNNeo
Copy link
Owner

imaNNeo commented Mar 11, 2022

Our branching approach has just changed. We had to remove the dev branch (Which was your base branch).
Check out our new branching approach here.

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.

5 participants