Floating-Dartists / matomo-tracker

A fully cross-platform wrap of the Matomo tracking client for Flutter, using the Matomo API.
https://pub.dev/packages/matomo_tracker
MIT License
26 stars 28 forks source link

[FEAT] Improve `matomoObserver` to dispense with `TraceableClientMixin`. #173

Closed lsaudon closed 1 month ago

lsaudon commented 1 month ago

Is your feature request related to a problem? Please describe.

We can extend of RouteObserver<PageRoute<dynamic>> to dispense with TraceableClientMixin.

Describe the solution you'd like

Proof of concept:

import 'package:flutter/material.dart';
import 'package:matomo_tracker/matomo_tracker.dart';

class MatomoObserver extends RouteObserver<PageRoute<dynamic>> {
  void _trackPageView(final Route<dynamic>? route) {
    MatomoTracker.instance.trackPageViewWithName(
      actionName: route?.settings.name ?? 'unknown',
    );
  }

  @override
  void didPush(
    final Route<dynamic> route,
    final Route<dynamic>? previousRoute,
  ) {
    super.didPush(route, previousRoute);
    _trackPageView(route);
  }

  @override
  void didPop(final Route<dynamic> route, final Route<dynamic>? previousRoute) {
    super.didPop(route, previousRoute);
    _trackPageView(previousRoute);
  }
}
TesteurManiak commented 1 month ago

I'm not sure to understand what is the link with TraceableClientMixin. 🤔 Your suggested implementation looks to me more like a global route observer that would track all page navigation in the app, where TraceableClientMixin is made to be used with specific widgets that you want to track in mind.

lsaudon commented 1 month ago

Yes, that's right. The goal is just to add the ability to track globally without having to use TraceableClientMixin everywhere.

TesteurManiak commented 1 month ago

Thanks for the clarification! I'm wondering if it is worth integrating this additional observer directly into the package, as showcased the implementation is only about 20 lines of code and the usage might be misleading alongside the existing matomoObserver. Maybe we could add a section in the documentation or an example in the repo with the code snippet you've provided to inform users of this technique to globally track navigation in their app. 🤔 (I'm open to suggestions, would love to have a second opinion on this)

lsaudon commented 1 month ago

I need to track all page changes. I think others have this need. The current solution makes this tedious. There may be a solution to merge the two. Track all pages and for certain widgets (pop up, etc.) use TraceableClientMixin.

TesteurManiak commented 1 month ago

I need to track all page changes. I think others have this need.

Sure, but there might also be people out there who needs to only track specific page.

The current solution makes this tedious.

And the implementation you've provided is short and doesn't rely on any kind of internal API of the package hence why I'm not sure that it needs to be integrated directly in it.

There may be a solution to merge the two. Track all pages and for certain widgets (pop up, etc.) use TraceableClientMixin.

As said above some devs might not want to track indiscriminately all pages in their apps, having more granularity in the tracking seems to me as important as providing a global observer. Because of this difference in behavior I'm not sure it is possible to mix the two, if anything is implemented a separate observer dedicated to TraceableClientMixin should be kept.

lsaudon commented 1 month ago

The observer can be configured to include or exclude path or name page.

The solution I have shown is simplistic for the moment, but it can be improved.

TesteurManiak commented 1 month ago

The observer can be configured to include or exclude path or name page.

The issue here is that any type of configuration will make the observer incompatible with TraceableClientMixin because it needs to point to specific observer's reference. (hence why the matomoObserver variable)

I think it's preferable to have a MatomoGlobalObserver (that would be instanciated by the dev, so it would be possible to customize some properties) and a matomoLocalObserver (used only by TraceableClientMixin).