Feat/chart callbacks #43

handgull · 2025-10-05T06:45:41Z

Please do not merge this.

Hi, this code isn’t working, but I gave it a try. Could you take a look at it? Honestly, I’ve never used this approach to call Dart code from JavaScript event handlers.
I’d like to take this opportunity to ask which other callbacks you consider useful to add at this point, besides the click event.

refactor GraphifyController and update dependencies

…fy-interface refactor: update plugin paths and improve Graphify interface structure

bump version to 1.2.0 in pubspec.yaml

…-wasm-it-reports-a-bug-unimplementederror-initview-is-not-implemented refactor: update conditional imports to use dart.library.js_interop

This commit removes Linux support from the `pubspec.yaml` file.

handgull · 2025-10-26T09:55:17Z

@warioddly now should be fine, sorry for the confusion. My project works well :)

warioddly · 2025-11-07T02:52:59Z

hi @handgull , it's great to see you again

huge thanks for the contribution, I'll check the PR when I'm free

Copilot

Pull Request Overview

This PR introduces a major refactoring of the Graphify Flutter charting library, adding chart click event handling, modernizing the architecture, and updating dependencies. The changes include significant API modifications that may impact existing users.

Key Changes:

Added chart click event support with chartClickedEvent stream on GraphifyController
Refactored view architecture to use inheritance instead of composition, with GraphifyView extending StatefulWidget
Updated to use dart.library.js_interop instead of deprecated dart.library.html for web platform detection

Reviewed Changes

Copilot reviewed 27 out of 29 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
pubspec.yaml	Version bump to 1.2.1, removed Linux platform support
lib/src/view/_interface.dart	Refactored GraphifyView to extend StatefulWidget, added @nonvirtual annotations
lib/src/view/implements/web.dart	Updated web implementation with click event shim and refactored to use inheritance
lib/src/view/implements/mobile.dart	Updated mobile implementation with JavaScript channel for click events
lib/src/view/implements/facade.dart	Updated facade to match new architecture
lib/src/controller/interface.dart	Added chartClickedEvent stream, removed Disposable mixin
lib/src/controller/implements/web.dart	Implemented click event handling with window message listener
lib/src/controller/implements/mobile.dart	Implemented click event handling with JavaScript channel
lib/src/utils/color_extension.dart	Refactored color-to-RGBA conversion (contains bugs)
lib/src/utils/uid.dart	New secure UID generator replacing Utils class
lib/src/resources/dependencies.js.dart	Added initClickListener JavaScript function for chart interactions
example/lib/charts/chart_click.dart	New example demonstrating chart click functionality
README.md	Fixed typos in GitHub URLs
CHANGELOG.md	Added 1.2.0 release notes
android/build.gradle	Updated Kotlin to 2.1.0 and Gradle to 8.7.0

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-07T14:42:34Z

lib/src/resources/dependencies.js.dart

+          function safeValue(value, depth) {
+            if (value == null) return null;
+            if (depth > 3) return undefined;
+            const t = typeof value;
+            if (t === 'string' || t === 'number' || t === 'boolean') return value;
+            if (Array.isArray(value)) return value.map(v => safeValue(v, depth + 1)).filter(v => v !== undefined);
+            if (t === 'object') {
+              const out = {};
+              const keys = ['type', 'componentType', 'seriesType', 'seriesIndex', 'seriesName', 'name', 'dataIndex', 'data', 'value', 'color', 'marker'];
+              keys.forEach(k => {
+                if (k in value) {
+                  const v = safeValue(value[k], depth + 1);
+                  if (v !== undefined) out[k] = v;
+                }
+              });
+              // fallback: pick plain props if 'data' is missing
+              if (!('data' in out) && value && typeof value === 'object') {
+                Object.keys(value).forEach(k => {
+                  if (out[k] !== undefined) return;
+                  const v = value[k];
+                  const vt = typeof v;
+                  if (vt === 'string' || vt === 'number' || vt === 'boolean') out[k] = v;
+                });
+              }
+              return out;
+            }
+            return undefined;
+          }


The safeValue function has a depth limit of 3 (line 41) but doesn't handle the case when depth exceeds 3 for non-primitive values. When depth > 3, it returns undefined, which is then filtered out in arrays (line 44). However, for objects, when a value at depth > 3 is encountered, undefined is stored in the object (line 51), which could lead to inconsistent data. Consider handling this more explicitly or documenting the depth limitation.

Copilot · 2025-11-07T14:42:34Z

pubspec.yaml

  android:
  ios:
-  linux:
  macos:


Linux platform support has been removed from the platforms list. If this is intentional, ensure that this breaking change is clearly documented in the CHANGELOG and that users are notified. If Linux was previously supported and is being removed, this should be a major version bump (2.0.0) rather than a minor version bump (1.2.1).

Copilot · 2025-11-07T14:42:35Z

lib/src/view/implements/web.dart

+      ..id = 'graphify-click-channel-shim-$uid'
+      ..innerHTML = '''
+      (function(uid){
+        // Definisci il channel solo se non già presente


The comment on line 78 is in Italian: "Definisci il channel solo se non già presente" (Define the channel only if not already present). Comments should be in English for consistency and accessibility to all contributors.

Suggested change

// Definisci il channel solo se non già presente

// Define the channel only if not already present

Copilot · 2025-11-07T14:42:35Z

lib/src/view/implements/web.dart

+                  chartId: uid,
+                  payload: String(payload)
+                });
+                // Inoltra come stringa: il controller web si aspetta un JSON string


The comment on line 88 is in Italian: "Inoltra come stringa: il controller web si aspetta un JSON string" (Forward as string: the web controller expects a JSON string). Comments should be in English for consistency and accessibility to all contributors.

Suggested change

// Inoltra come stringa: il controller web si aspetta un JSON string

// Forward as string: the web controller expects a JSON string

Copilot · 2025-11-07T14:42:35Z

lib/src/controller/implements/mobile.dart

+  void dispose() async {
+    await _clicks.close();
+    try {
+      await _eval(JsMethods.disposeChart, [_quotedUid]);
+    } catch (_) {}
  }


The dispose() method is declared with void return type but contains an await statement on line 28. This creates a "fire-and-forget" async operation that could lead to incomplete cleanup. Change the return type to Future<void> and ensure callers properly await the disposal.

Copilot · 2025-11-07T14:42:36Z

lib/src/utils/color_extension.dart

+    final r255 = (r * 255.0).round() & 0xff;
+    final g255 = (g * 255.0).round() & 0xff;
+    final b255 = (b * 255.0).round() & 0xff;
+    final alpha = a; // 0..1


The new implementation references properties r, g, b, and a that don't exist on the Color class in Dart. The Color class uses red, green, blue, and opacity properties (as 0-255 integers for RGB and 0.0-1.0 for opacity). This code will fail at runtime. Use the correct property names: red, green, blue, and opacity.

Suggested change

final r255 = (r * 255.0).round() & 0xff;

final g255 = (g * 255.0).round() & 0xff;

final b255 = (b * 255.0).round() & 0xff;

final alpha = a; // 0..1

final r255 = red;

final g255 = green;

final b255 = blue;

final alpha = opacity; // 0..1

Copilot · 2025-11-07T14:42:36Z

lib/src/view/implements/web.dart

-      });
+      ..srcdoc = indexHtml(id: uid).toJS
+      ..onLoad.listen((_) => controller.update(widget.initialOptions))
+      ..onError.listen(widget.onConsoleMessage);


The onConsoleMessage callback signature has changed from void Function(String message) to void Function(Object message). In the web implementation, the error event is now passed directly without converting to string. This could break existing code that expects a String parameter. Consider maintaining backward compatibility or documenting this breaking change more clearly.

Suggested change

..onError.listen(widget.onConsoleMessage);

..onError.listen((event) => widget.onConsoleMessage?.call(event.toString()));

Copilot · 2025-11-07T14:42:37Z

lib/src/view/implements/mobile.dart

+    webViewController
+      ..clearLocalStorage()
+      ..clearCache();


The webViewController cleanup operations (clearLocalStorage() and clearCache()) are chained without awaiting. If these are async operations, this could lead to incomplete cleanup before the widget is fully disposed. Consider awaiting these operations if they return futures.

Copilot · 2025-11-07T14:42:37Z

example/lib/charts/basic_line_chart.dart

    return GraphifyView(
      controller: controller,
-      onConsoleMessage: (message) {
+      onConsoleMessage: <String>(message) {


The type annotation <String> on line 38 is inconsistent with the new OnConsoleMessage signature which expects Object message. This will cause a type mismatch error. The parameter type should be Object or removed to allow type inference.

Suggested change

onConsoleMessage: <String>(message) {

onConsoleMessage: (message) {

Copilot · 2025-11-07T14:42:37Z

lib/src/controller/implements/facade.dart

+  Stream<Map<String, dynamic>> get chartClickedEvent =>
+      throw UnimplementedError("chartClickedEvent is not implemented");


The chartClickedEvent getter throws an UnimplementedError instead of returning a Stream. This will cause runtime errors when accessed. Consider returning a never-emitting Stream (e.g., Stream.empty()) or a closed StreamController's stream to maintain consistent API behavior.

Suggested change

Stream<Map<String, dynamic>> get chartClickedEvent =>

throw UnimplementedError("chartClickedEvent is not implemented");

Stream<Map<String, dynamic>> get chartClickedEvent => Stream.empty();

warioddly and others added 13 commits July 4, 2025 23:42

Merge pull request warioddly#36 from warioddly/dev

4f1137d

refactor GraphifyController and update dependencies

refactor: update plugin paths and improve Graphify interface structure

ca35f96

Merge pull request warioddly#37 from warioddly/feature/improve-graphi…

41b680f

…fy-interface refactor: update plugin paths and improve Graphify interface structure

bump version to 1.3.0 in pubspec.yaml

2f74c7c

bump version to 1.2.0 in pubspec.yaml and pubspec.lock

a0d35a0

Merge pull request warioddly#38 from warioddly/feature/bump-version

169a859

bump version to 1.2.0 in pubspec.yaml

chore: update CHANGELOG.md for version 1.2.0 release

24ff7de

Merge remote-tracking branch 'origin/main'

02549da

refactor: update conditional imports to use dart.library.js_interop

0e77db5

Merge pull request warioddly#41 from warioddly/40-when-running-with--…

4a98d98

…-wasm-it-reports-a-bug-unimplementederror-initview-is-not-implemented refactor: update conditional imports to use dart.library.js_interop

bump version to 1.2.1 in pubspec.yaml

b819eea

feat: remove linux platform support

7629314

This commit removes Linux support from the `pubspec.yaml` file.

new click feature testing setup + gradle bump

e34face

handgull force-pushed the feat/chart-callbacks branch 6 times, most recently from 130f4fa to fa59e22 Compare October 26, 2025 09:28

chart data sent when tapped

6f4c6bb

handgull force-pushed the feat/chart-callbacks branch from fa59e22 to 6f4c6bb Compare October 26, 2025 09:54

added web implementation

5421b50

warioddly requested review from Copilot and warioddly November 7, 2025 14:38

warioddly assigned handgull Nov 7, 2025

warioddly added the enhancement New feature or request label Nov 7, 2025

Copilot AI reviewed Nov 7, 2025

View reviewed changes

some review fixes

cb62649

handgull force-pushed the feat/chart-callbacks branch from 3766ac8 to cb62649 Compare November 12, 2025 08:52

handgull added 2 commits November 20, 2025 12:18

removed print

1b10de3

remove print

9b9db1e

	// Definisci il channel solo se non già presente
	// Define the channel only if not already present

	// Inoltra come stringa: il controller web si aspetta un JSON string
	// Forward as string: the web controller expects a JSON string

	..onError.listen(widget.onConsoleMessage);
	..onError.listen((event) => widget.onConsoleMessage?.call(event.toString()));

	onConsoleMessage: <String>(message) {
	onConsoleMessage: (message) {

		Stream<Map<String, dynamic>> get chartClickedEvent =>
		throw UnimplementedError("chartClickedEvent is not implemented");

Feat/chart callbacks #43

Are you sure you want to change the base?

Feat/chart callbacks #43

Uh oh!

Conversation

handgull commented Oct 5, 2025

Uh oh!

handgull commented Oct 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

warioddly commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

handgull commented Oct 26, 2025 •

edited

Loading

warioddly commented Nov 7, 2025 •

edited

Loading