google/charted

`ChartData` creates its own `rows` object

benasocj opened this issue · 0 comments

8eecd1a has broken the examples/charts/demo_interactive.dart example. Clicking the "Add row" or "Remove last row" buttons no longer shows any action. This is because of this patch in the commit mentioned above:

diff --git a/lib/charts/src/chart_data_impl.dart b/lib/charts/src/chart_data_impl.dart
index dff6f5c..2dc6ed0 100644
--- a/lib/charts/src/chart_data_impl.dart
+++ b/lib/charts/src/chart_data_impl.dart
@@ -9,16 +9,18 @@
 part of charted.charts;

 class DefaultChartDataImpl extends ChangeNotifier implements ChartData {
-  Iterable<ChartColumnSpec> _columns;
-  Iterable<Iterable> _rows;
+  List<ChartColumnSpec> _columns;
+  List<List> _rows;

   bool _hasObservableRows = false;
   SubscriptionsDisposer _disposer = new SubscriptionsDisposer();

   DefaultChartDataImpl(
       Iterable<ChartColumnSpec> columns, Iterable<Iterable> rows) {
-    this.columns = columns;
-    this.rows = rows;
+    this.columns = new List<ChartColumnSpec>.from(columns);
+    var rowsList = new List.from(rows);
+    this.rows = new List<List>.generate(
+        rowsList.length, (i) => new List.from(rowsList[i]));
   }

   set columns(Iterable<ChartColumnSpec> value) {

The problem is that the rows object which is passed to the DefaultChartDataImpl constructor is no longer stored in this.rows. Instead, a new List object is generated which is then stored in this.rows.

In my opinion it would be great if the original behaviour could be restored as with the current state we can no longer do this:

ChartData data = new ChartData(columns, rows);

// Do other stuff

// New rows have been received. Let's add them.
rows.add(...);
// Doesn't work since `ChartData` has created its own `rows` object internally :(.

// We can use this workaround but is not that elegant.
data.rows.add(...);

Also, this doesn't work anymore and always returns false now since the new rows object is created without any regard whether the original rows object was an instance of ObservableList or not.