Fork ChangeNotifier

The EditorStates are inheriting from State and using the ChangeNotifier
mixin. Both the ChangeNotifier and the State class have a dispose
function, which needs to be call.

Without this only the ChangeNotifier's dispose method was being called.

lib/editors/checklist_editor.dart

@@ -9,6 +9,7 @@ import 'package:easy_localization/easy_localization.dart';
 import 'package:gitjournal/core/checklist.dart';
 import 'package:gitjournal/core/note.dart';
 import 'package:gitjournal/editors/common.dart';
+import 'package:gitjournal/editors/disposable_change_notifier.dart';
 import 'package:gitjournal/editors/note_title_editor.dart';
 
 class ChecklistEditor extends StatefulWidget implements Editor {
@@ -53,7 +54,7 @@ class ChecklistEditor extends StatefulWidget implements Editor {
 }
 
 class ChecklistEditorState extends State<ChecklistEditor>
-    with ChangeNotifier
+    with DisposableChangeNotifier
     implements EditorState {
   Checklist checklist;
   var focusNodes = <UniqueKey, FocusScopeNode>{};
@@ -84,6 +85,8 @@ class ChecklistEditorState extends State<ChecklistEditor>
   @override
   void dispose() {
     _titleTextController.dispose();
+
+    super.disposeListenables();
     super.dispose();
   }
 

lib/editors/disposable_change_notifier.dart

@@ -0,0 +1,134 @@
+import 'package:flutter/foundation.dart';
+import 'package:flutter/material.dart';
+
+/// A duplicate of the ChangeNotifier class which has the dispose method
+/// renamed to disposeListeners. This is done so that it can be used
+/// as a mixin with a State class which also has a dispose method
+class DisposableChangeNotifier implements Listenable {
+  ObserverList<VoidCallback> _listeners = ObserverList<VoidCallback>();
+
+  bool _debugAssertNotDisposed() {
+    assert(() {
+      if (_listeners == null) {
+        throw FlutterError('A $runtimeType was used after being disposed.\n'
+            'Once you have called dispose() on a $runtimeType, it can no longer be used.');
+      }
+      return true;
+    }());
+    return true;
+  }
+
+  /// Whether any listeners are currently registered.
+  ///
+  /// Clients should not depend on this value for their behavior, because having
+  /// one listener's logic change when another listener happens to start or stop
+  /// listening will lead to extremely hard-to-track bugs. Subclasses might use
+  /// this information to determine whether to do any work when there are no
+  /// listeners, however; for example, resuming a [Stream] when a listener is
+  /// added and pausing it when a listener is removed.
+  ///
+  /// Typically this is used by overriding [addListener], checking if
+  /// [hasListeners] is false before calling `super.addListener()`, and if so,
+  /// starting whatever work is needed to determine when to call
+  /// [notifyListeners]; and similarly, by overriding [removeListener], checking
+  /// if [hasListeners] is false after calling `super.removeListener()`, and if
+  /// so, stopping that same work.
+  @protected
+  bool get hasListeners {
+    assert(_debugAssertNotDisposed());
+    return _listeners.isNotEmpty;
+  }
+
+  /// Register a closure to be called when the object changes.
+  ///
+  /// This method must not be called after [dispose] has been called.
+  @override
+  void addListener(VoidCallback listener) {
+    assert(_debugAssertNotDisposed());
+    _listeners.add(listener);
+  }
+
+  /// Remove a previously registered closure from the list of closures that are
+  /// notified when the object changes.
+  ///
+  /// If the given listener is not registered, the call is ignored.
+  ///
+  /// This method must not be called after [dispose] has been called.
+  ///
+  /// If a listener had been added twice, and is removed once during an
+  /// iteration (i.e. in response to a notification), it will still be called
+  /// again. If, on the other hand, it is removed as many times as it was
+  /// registered, then it will no longer be called. This odd behavior is the
+  /// result of the [ChangeNotifier] not being able to determine which listener
+  /// is being removed, since they are identical, and therefore conservatively
+  /// still calling all the listeners when it knows that any are still
+  /// registered.
+  ///
+  /// This surprising behavior can be unexpectedly observed when registering a
+  /// listener on two separate objects which are both forwarding all
+  /// registrations to a common upstream object.
+  @override
+  void removeListener(VoidCallback listener) {
+    assert(_debugAssertNotDisposed());
+    _listeners.remove(listener);
+  }
+
+  /// Discards any resources used by the object. After this is called, the
+  /// object is not in a usable state and should be discarded (calls to
+  /// [addListener] and [removeListener] will throw after the object is
+  /// disposed).
+  ///
+  /// This method should only be called by the object's owner.
+  @mustCallSuper
+  void disposeListenables() {
+    assert(_debugAssertNotDisposed());
+    _listeners = null;
+  }
+
+  /// Call all the registered listeners.
+  ///
+  /// Call this method whenever the object changes, to notify any clients the
+  /// object may have. Listeners that are added during this iteration will not
+  /// be visited. Listeners that are removed during this iteration will not be
+  /// visited after they are removed.
+  ///
+  /// Exceptions thrown by listeners will be caught and reported using
+  /// [FlutterError.reportError].
+  ///
+  /// This method must not be called after [dispose] has been called.
+  ///
+  /// Surprising behavior can result when reentrantly removing a listener (i.e.
+  /// in response to a notification) that has been registered multiple times.
+  /// See the discussion at [removeListener].
+  @protected
+  @visibleForTesting
+  void notifyListeners() {
+    assert(_debugAssertNotDisposed());
+    if (_listeners != null) {
+      final List<VoidCallback> localListeners =
+          List<VoidCallback>.from(_listeners);
+      for (final VoidCallback listener in localListeners) {
+        try {
+          if (_listeners.contains(listener)) {
+            listener();
+          }
+        } catch (exception, stack) {
+          FlutterError.reportError(FlutterErrorDetails(
+            exception: exception,
+            stack: stack,
+            library: 'foundation library',
+            context: ErrorDescription(
+                'while dispatching notifications for $runtimeType'),
+            informationCollector: () sync* {
+              yield DiagnosticsProperty<DisposableChangeNotifier>(
+                'The $runtimeType sending notification was',
+                this,
+                style: DiagnosticsTreeStyle.errorProperty,
+              );
+            },
+          ));
+        }
+      }
+    }
+  }
+}

lib/editors/journal_editor.dart

@@ -4,6 +4,7 @@ import 'package:flutter/material.dart';
 
 import 'package:gitjournal/core/note.dart';
 import 'package:gitjournal/editors/common.dart';
+import 'package:gitjournal/editors/disposable_change_notifier.dart';
 import 'package:gitjournal/widgets/editor_scroll_view.dart';
 import 'package:gitjournal/widgets/journal_editor_header.dart';
 
@@ -49,7 +50,7 @@ class JournalEditor extends StatefulWidget implements Editor {
 }
 
 class JournalEditorState extends State<JournalEditor>
-    with ChangeNotifier
+    with DisposableChangeNotifier
     implements EditorState {
   Note note;
   TextEditingController _textController = TextEditingController();
@@ -68,6 +69,8 @@ class JournalEditorState extends State<JournalEditor>
   @override
   void dispose() {
     _textController.dispose();
+
+    super.disposeListenables();
     super.dispose();
   }
 

lib/editors/markdown_editor.dart

@@ -4,6 +4,7 @@ import 'package:flutter/material.dart';
 
 import 'package:gitjournal/core/note.dart';
 import 'package:gitjournal/editors/common.dart';
+import 'package:gitjournal/editors/disposable_change_notifier.dart';
 import 'package:gitjournal/editors/heuristics.dart';
 import 'package:gitjournal/editors/note_title_editor.dart';
 import 'package:gitjournal/error_reporting.dart';
@@ -54,7 +55,7 @@ class MarkdownEditor extends StatefulWidget implements Editor {
 }
 
 class MarkdownEditorState extends State<MarkdownEditor>
-    with ChangeNotifier
+    with DisposableChangeNotifier
     implements EditorState {
   Note note;
   TextEditingController _textController = TextEditingController();
@@ -93,6 +94,8 @@ class MarkdownEditorState extends State<MarkdownEditor>
   void dispose() {
     _textController.dispose();
     _titleTextController.dispose();
+
+    super.disposeListenables();
     super.dispose();
   }
 

lib/editors/raw_editor.dart

@@ -5,6 +5,7 @@ import 'package:flutter/material.dart';
 import 'package:gitjournal/core/md_yaml_doc_codec.dart';
 import 'package:gitjournal/core/note.dart';
 import 'package:gitjournal/editors/common.dart';
+import 'package:gitjournal/editors/disposable_change_notifier.dart';
 import 'package:gitjournal/widgets/editor_scroll_view.dart';
 
 class RawEditor extends StatefulWidget implements Editor {
@@ -49,7 +50,7 @@ class RawEditor extends StatefulWidget implements Editor {
 }
 
 class RawEditorState extends State<RawEditor>
-    with ChangeNotifier
+    with DisposableChangeNotifier
     implements EditorState {
   Note note;
   bool _noteModified;
@@ -70,6 +71,8 @@ class RawEditorState extends State<RawEditor>
   @override
   void dispose() {
     _textController.dispose();
+
+    super.disposeListenables();
     super.dispose();
   }