Merge pull request #227 from woltapp/dispose-value-notifier-listener

Dispose value notifier in WoltModalSheet
woltapp · Jun 5, 2024 · 6c16db9 · 6c16db9
2 parents b8b1dc0 + 13afb0e
commit 6c16db9
Show file tree

Hide file tree

Showing 2 changed files with 106 additions and 12 deletions.
diff --git a/lib/src/wolt_modal_sheet.dart b/lib/src/wolt_modal_sheet.dart
@@ -352,18 +352,15 @@ class WoltModalSheetState extends State<WoltModalSheet> {
   @override
   void initState() {
     super.initState();
-    widget.pageListBuilderNotifier.addListener(() {
-      // Update the page list whenever the notifier changes. This is useful when the page list is
-      // set in a declarative way using the WoltModalSheetRoute.
-      setState(() {
-        final pages = widget.pageListBuilderNotifier.value(context);
-        assert(
-            pages.isNotEmpty, 'pageListBuilder must return a non-empty list.');
-        _pages
-          ..clear()
-          ..addAll(pages);
-      });
-    });
+    widget.pageListBuilderNotifier
+        .addListener(_onPageListBuilderNotifierValueUpdated);
+  }
+
+  @override
+  void dispose() {
+    widget.pageListBuilderNotifier
+        .removeListener(_onPageListBuilderNotifierValueUpdated);
+    super.dispose();
   }
 
   @override
@@ -534,6 +531,19 @@ class WoltModalSheetState extends State<WoltModalSheet> {
     );
   }
 
+  void _onPageListBuilderNotifierValueUpdated() {
+    if (context.mounted) {
+      setState(() {
+        final pages = widget.pageListBuilderNotifier.value(context);
+        assert(
+            pages.isNotEmpty, 'pageListBuilder must return a non-empty list.');
+        _pages
+          ..clear()
+          ..addAll(pages);
+      });
+    }
+  }
+
   void _handleDragUpdate(DragUpdateDetails details) {
     if (_dismissUnderway) {
       return;

diff --git a/test/wolt_modal_sheet_test.dart b/test/wolt_modal_sheet_test.dart
@@ -65,6 +65,90 @@ void main() {
     });
   });
 
+  group('WoltModalSheet State Management', () {
+    testWidgets('should update pages when notifier changes',
+        (WidgetTester tester) async {
+      final pageListBuilderNotifier = ValueNotifier((BuildContext _) => [
+            WoltModalSheetPage(
+              child: const Text('Initial Page'),
+            ),
+          ]);
+
+      await tester.pumpWidget(
+        MaterialApp(
+          home: WoltModalSheet(
+            pageListBuilderNotifier: pageListBuilderNotifier,
+            pageIndexNotifier: ValueNotifier(0),
+            onModalDismissedWithBarrierTap: () {},
+            onModalDismissedWithDrag: () {},
+            decorator: null,
+            modalTypeBuilder: (_) => WoltModalType.bottomSheet,
+            animationController: null,
+            route: WoltModalSheetRoute<void>(
+                pageListBuilderNotifier: pageListBuilderNotifier),
+            enableDrag: null,
+            showDragHandle: null,
+            useSafeArea: false,
+          ),
+        ),
+      );
+
+      // Initial state check
+      expect(find.text('Initial Page'), findsOneWidget);
+
+      // Update the notifier
+      pageListBuilderNotifier.value = (_) => [
+            WoltModalSheetPage(child: const Text('Updated Page')),
+          ];
+
+      // Trigger the listener
+      pageListBuilderNotifier.notifyListeners();
+      await tester.pumpAndSettle();
+
+      // Check if the UI is updated
+      expect(find.text('Updated Page'), findsOneWidget);
+    });
+
+    testWidgets('listener should be removed on dispose',
+        (WidgetTester tester) async {
+      final pageListBuilderNotifier = ValueNotifier((_) => [
+            WoltModalSheetPage(child: const Text('Initial Page')),
+          ]);
+
+      await tester.pumpWidget(
+        MaterialApp(
+          home: WoltModalSheet(
+            pageListBuilderNotifier: pageListBuilderNotifier,
+            pageIndexNotifier: ValueNotifier(0),
+            onModalDismissedWithBarrierTap: () {},
+            onModalDismissedWithDrag: () {},
+            decorator: null,
+            modalTypeBuilder: (_) => WoltModalType.bottomSheet,
+            animationController: null,
+            route: WoltModalSheetRoute<void>(
+                pageListBuilderNotifier: pageListBuilderNotifier),
+            enableDrag: null,
+            showDragHandle: null,
+            useSafeArea: false,
+          ),
+        ),
+      );
+
+      // Update the notifier after the widget is disposed
+      await tester.pumpWidget(Container()); // Dispose the widget
+      pageListBuilderNotifier.value = (_) => [
+            WoltModalSheetPage(child: const Text('Should Not Update')),
+          ];
+
+      // Trigger the listener
+      pageListBuilderNotifier.notifyListeners();
+      await tester.pumpAndSettle();
+
+      // Since the widget is disposed, this text should not be found
+      expect(find.text('Should Not Update'), findsNothing);
+    });
+  });
+
   group('barrierDismissible', () {
     testWidgets(
         'Does not dismiss on barrier tap if barrierDismissible is false',