Reentry and extended example

[EPNW-Eric]

To associate an event with a page view, trackEvent should take a pvId.

Next, there should be a concept of "reentry": If we push a page on top of a page that extends TraceableClientMixin and then pop back again, we need to tell Matomo this, since initState is not called again in such a case. Therfor, I added a onReentry method with an optional parameter. This parameter can be used to control if this reentry is conceptually a new page view in an apps lifecycle or not. (The new example demonstrates that). The first commit in this PR deals with both of this things.

The second commit extends the example a bit and also adds instructions on how to run a local Matomo instance for easy testing using docker.

[TesteurManiak]

Thanks a lot for this huge contribution!

I've quickly checked your first commit that implement reentry, I'm wondering if you can automatically call the method by overriding one of the lifecycle methods of the state instead of the developer having to do it manually 🤔 (Otherwise the rest of the commit LGTM)

I'll try to review the rest of the changes as soon as I can. (When there's that much changes it would've been better to make 2 separate PRs to facilitate our review process 😅 )

[EPNW-Eric]

Thanks a lot for this huge contribution!

Its actually not that huge, the second commit only has so many changes since I run flutter create to reinitalize the example project to bump the versions 😉 The only thing that really changed there was lib/main.dart, README.md (but that was trivial), and the additions in the docker folder.

I've quickly checked your first commit that implement reentry, I'm wondering if you can automatically call the method by overriding one of the lifecycle methods of the state instead of the developer having to do it manually 🤔 (Otherwise the rest of the commit LGTM)

This was actually my first though too, but I do not think that any lifecycle method is appropriate here... But maybe I'm missing something, so I'm open to suggestions 😄

[TesteurManiak]

This was actually my first though too, but I do not think that any lifecycle method is appropriate here... But maybe I'm missing something, so I'm open to suggestions 😄

I'll do some testing on my side to see if it can be simplified 👍

[TesteurManiak]

Also I didn't see that this PR was targeting the branch Floating-Dartists:refactor/#65_update-parameters, it should actually target the dev branch after #68 is merged

[TesteurManiak]

Okay I've found a way that will help to automatically call onReentry:

traceable_widget_mixin.dart

mixin TraceableClientMixin<T extends StatefulWidget> on State<T> {
  // ...

  /// Replacement for the build method to use in a tracked widget.
  @protected
  Widget buildTraceableWidget(BuildContext context);

  /// Override of the base build method to inject a widget that will listen to page
  /// changes and will trigger the call to onReentry.
  @override
  Widget build(BuildContext context) {
    return RouteLifecycleListener(
      didPopNext: () {
        // Should call onReentry
      },
      child: buildTraceableWidget(context),
    );
  }
}

route_lifecycle_listener.dart

final matomoObserver = RouteObserver<ModalRoute<void>>();

class RouteLifecycleListener extends StatefulWidget {
  const RouteLifecycleListener({
    super.key,
    this.didPopNext,
    this.didPush,
    this.didPop,
    this.didPushNext,
    required this.child,
  });

  /// The next screen (covering the current one) has been popped.
  /// This screen is now the current one.
  final VoidCallback? didPopNext;

  /// Called when the current route has been pushed.
  final VoidCallback? didPush;

  /// Called when the current route has been popped off.
  final VoidCallback? didPop;

  /// Called when a new route has been pushed, and the current route is no
  /// longer visible.
  final VoidCallback? didPushNext;

  final Widget child;

  @override
  State<RouteLifecycleListener> createState() => _RouteLifecycleListenerState();
}

class _RouteLifecycleListenerState extends State<RouteLifecycleListener>
    implements RouteAware {
  @override
  void didChangeDependencies() {
    super.didChangeDependencies();
    matomoObserver.subscribe(this, ModalRoute.of(context)!);
  }

  @override
  void dispose() {
    matomoObserver.unsubscribe(this);
    super.dispose();
  }

  @override
  Widget build(BuildContext context) {
    return widget.child;
  }

  @override
  void didPop() => widget.didPop?.call();

  @override
  void didPopNext() => widget.didPopNext?.call();

  @override
  void didPush() => widget.didPush?.call();

  @override
  void didPushNext() => widget.didPushNext?.call();
}

You will just have to register matomoObserver in your navigatorObservers:

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Matomo Example',
      home: const MyHomePage(title: 'Matomo Example'),
      navigatorObservers: [matomoObserver],
    );
  }
}

[TesteurManiak]

(I can add the code directly to the PR #68 if it's easier for you as it implies to update a few widget test)

[EPNW-Eric]

That seems to be a greate solution! I wonder if TraceableClientMixin could implement RouteAware directly instead of using a proxy widget? Unfortunately I don't have any more time today, so feel free to take the necessary steps yourself 😄 Otherwise I can look into it tomorrow.

[Pierre-Monier]

That seems to be a greate solution! I wonder if TraceableClientMixin could implement RouteAware directly instead of using a proxy widget? Unfortunately I don't have any more time today, so feel free to take the necessary steps yourself 😄 Otherwise I can look into it tomorrow.

Even if it can, it might be a better design to separate the RouteAware part in a proxy widget. This way we can have more separation

[TesteurManiak]

That seems to be a greate solution! I wonder if TraceableClientMixin could implement RouteAware directly instead of using a proxy widget?

Yeah it might actually be possible by implementing RouteAware directly in the mixin. I'll add those changes on the dev branch directly as it shouldn't cause any regression with a comment mentioning this issue so you'll know where to add your call to onReentry.

[TesteurManiak]

@EPNW-Eric same here you can re-open the PR by targeting the dev branch, sorry for the inconvenience 😅

Even if it can, it might be a better design to separate the RouteAware part in a proxy widget. This way we can have more separation

I don't know about that. We really just use the didPopNext callback and don't need a full reusable widget, it's all that much more code to test and maintain 🤔

[Pierre-Monier]

I don't know about that. We really just use the didPopNext callback and don't need a full reusable widget, it's all that much more code to test and maintain 🤔

If it's just that, let's write it directly in the mixin. If one day we have more related logic, let's put it in one place like the proxy widget

[TesteurManiak]

Route logic added in #73