google / charts

https://pub.dev/packages/charts_flutter
Apache License 2.0
2.81k stars 1.23k forks source link

Color vs charts.Color #130

Open moraleja39 opened 6 years ago

moraleja39 commented 6 years ago

I can not see any advantage of re-implementing the Color class just for this library. Given that it is a Flutter library, it seems more logical to use the Color class from the Flutter painting library.

In my use cases, I already had defined several color constants from the brand identity, and having to reinstantiate them for the charts felt rather bothersome.

Having the fromHex factory could seem handy, but I really can't see any advantage over using a base 16 int.

Any thoughts on this? Would be a non-breaking PR welcome?

lorrainekan commented 6 years ago

We totally understand this pain point. But in order to keep a majority of the logic as common Dart code, we had to create a separate Color class.

JamesMcIntosh commented 6 years ago

I've added utility methods to charts_common/lib/src/common/color.dart to convert between the two color classes in this pull request: https://github.com/google/charts/pull/126/files#diff-07b83f21249d0fa683b591f6fb93717e

JamesMcIntosh commented 6 years ago

Have a look at the color util class in the link of my previous comment

ozexpert commented 5 years ago

for workaround, i did like this

charts.Color.fromHex(code: '#f2f2f2')
TomOerlemans commented 5 years ago

Alternatively, you can use this function:

import 'package:charts_flutter/flutter.dart' as charts;

charts.Color getChartColor(Color color) {
    return charts.Color(
        r: color.red,
        g: color.green,
        b: color.blue,
        a: color.alpha);
}

e.g.

getChartColor(Colors.red)

wabash9000 commented 4 years ago

This basically broke my whole project. It's gave me errors in files that don't even have the package imported. I had to add a second import of the material class with "import 'package:flutter/material.dart' as mat;" to any file that used the Color class and then change every call to mat.Color. This is the worst design decision I have ever seen in a package. I don't mind you making a custom color class, but name it something different than Color. CColor, ChartColor, or ThisIsTheColorClassForChartsFlutter all would have been better. The fact that you have to have "as chart" and then "chart." on every call is absurd. I have never seen this on any other package.