[Breaking Change] Refactor page navigation in page list, remove pageIndexNotifier and pageListBuilderNotifier #182

ulusoyca · 2024-04-19T16:36:39Z

Overview

This pull request introduces breaking and major changes to the in-modal navigation system of the WoltModalSheet. The changes are aimed at increasing flexibility and improving the developer experience by allowing dynamic navigation updates, multiple page management, and a more intuitive interaction model for in-modal navigation.

Key Changes

Dynamic Page Addition and Management:
- addPagesToStack and its overload addPageToStack: These methods facilitate the addition of one or multiple pages to the navigation stack without altering while staying at current page. This is particularly useful for preloading pages in scenarios where the user might need to interact with multiple subsequent screens.

addAndReplace.mov

pushPages and its overload pushPage: Extended to allow pushing a list of pages onto the navigation stack, making the first of these new pages the currently visible one. This is crucial for workflows where a sequence of new pages needs to be added and immediately accessed.

push.mov

Page Replacement Enhancements:
- addOrReplacePages and addOrReplacePage: Updated methods to dynamically append new pages or replace existing pages beyond the current one in the navigation stack. This feature supports dynamic user journeys where decisions on earlier pages might change the subsequent path.
- replaceCurrentPage: A new method that replaces the current page in the stack without disturbing other pages, ensuring a smooth transition and immediate update of content without full navigation stack refresh.
- replaceAllPages: Replaces all pages in the stack while attempting to maintain the relative position of the current page, thus ensuring continuity in the user's navigation context.
Navigation Control Improvements:
- showNext and showPrevious: Methods to navigate forward or backward in the modal sheet stack, enhanced to handle edge cases and ensure that navigation commands respect the bounds of the page list.
State Management and Error Handling:
- Enhanced state management to ensure robust handling of navigation state across asynchronous operations. This includes checks to ensure operations are only performed when the widget context is active, preventing errors related to accessing deactivated widget ancestors.

Migration Notes

TODO

Testing

TODO

Future Work

TODO
I read the Contributor Guide and followed the process outlined there for submitting PRs.
My PR includes tests for all changed/updated/fixed behaviors.
All existing and new tests are passing.
I updated/added relevant documentation (doc comments with ///).
The analyzer (melos run analyze) does not report any problems on my PR.
The package compiles with the minimum Flutter version stated in the pubspec.yaml

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

Yes, this is a breaking change.
No, this is not a breaking change.

github-actions · 2024-04-19T16:39:54Z

Visit the preview URL for this PR (updated for commit d2b02bb):

_{(expires Fri, 26 Apr 2024 16:47:34 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: 906758393beb0353b979d020649d6a1efc40fb5b}

TahaTesser

Nice work!

TahaTesser · 2024-04-19T19:39:13Z

lib/src/wolt_modal_sheet.dart

+  /// This method can be expensive (it walks the element tree).
+  static WoltModalSheetState of(BuildContext context) {
+    // Handles the case where the input context is a navigator element.
+    WoltModalSheetState? wms;


This isn't public but i suggest to use words as they're readable and searchable if needed.

Suggested change

WoltModalSheetState? wms;

WoltModalSheetState? woltModalSheetState;

TahaTesser · 2024-04-19T19:39:47Z

lib/src/wolt_modal_sheet.dart

+    // Handles the case where the input context is a navigator element.
+    WoltModalSheetState? wms;
+    if (context is StatefulElement && context.state is WoltModalSheetState) {
+      wms = context.state as WoltModalSheetState;


Suggested change

wms = context.state as WoltModalSheetState;

woltModalSheetState = context.state as WoltModalSheetState;

TahaTesser · 2024-04-19T19:39:54Z

lib/src/wolt_modal_sheet.dart

+    if (context is StatefulElement && context.state is WoltModalSheetState) {
+      wms = context.state as WoltModalSheetState;
+    }
+    wms ??= context.findAncestorStateOfType<WoltModalSheetState>();


Suggested change

wms ??= context.findAncestorStateOfType<WoltModalSheetState>();

woltModalSheetState ??= context.findAncestorStateOfType<WoltModalSheetState>();

TahaTesser · 2024-04-19T19:40:01Z

lib/src/wolt_modal_sheet.dart

+    wms ??= context.findAncestorStateOfType<WoltModalSheetState>();
+
+    assert(() {
+      if (wms == null) {


Suggested change

if (wms == null) {

if (woltModalSheetState == null) {

TahaTesser · 2024-04-19T19:40:25Z

lib/src/wolt_modal_sheet.dart

+      }
+      return true;
+    }());
+    return wms!;


Suggested change

return wms!;

return woltModalSheetState!;

TahaTesser · 2024-04-19T19:54:53Z

lib/src/wolt_modal_sheet.dart

+    bool found = false;
+    final List<SliverWoltModalSheetPage> newPages = _pages.map((p) {
+      if (p.id == id) {
+        found = true;
+        return page;
+      }
+      return p;
+    }).toList();
+    if (!found) return false;


Suggested change

bool found = false;

final List<SliverWoltModalSheetPage> newPages = _pages.map((p) {

if (p.id == id) {

found = true;

return page;

}

return p;

}).toList();

if (!found) return false;

final List<SliverWoltModalSheetPage> newPages = _pages.where((SliverWoltModalSheetPage sheetPage) {

return sheetPage.id == id;

}).toList();

if (newPages.isEmpty) {

return false;

}

This could where and avoid using inline veriables. I haven't tested the code so please verify it works.

TahaTesser · 2024-04-19T19:55:14Z

lib/src/wolt_modal_sheet.dart

+  void pushPages(List<SliverWoltModalSheetPage> pages) {
+    setState(() {


Suggested change

void pushPages(List<SliverWoltModalSheetPage> pages) {

setState(() {

void pushPages(List<SliverWoltModalSheetPage> pages) {

if (pages.isEmpty) {

return;

}

setState(() {

if pages is empty, avoid calling setState.

TahaTesser · 2024-04-19T19:55:33Z

lib/src/wolt_modal_sheet.dart

+  ///
+  /// Returns:
+  /// None.
+  void replaceAllPages(List<SliverWoltModalSheetPage> newPages) {


Suggested change

void replaceAllPages(List<SliverWoltModalSheetPage> newPages) {

void replaceAllPages(List<SliverWoltModalSheetPage> newPages) {

if (newPages.isEmpty) {

return;

}

TahaTesser · 2024-04-19T19:57:25Z

lib/src/wolt_modal_sheet.dart

+      if (pages.isNotEmpty) {
+        _selectedPageIndex =
+            _pages.length - pages.length; // Set to the first of the newly added pages.
+      }


Suggested change

if (pages.isNotEmpty) {

_selectedPageIndex =

_pages.length - pages.length; // Set to the first of the newly added pages.

}

_selectedPageIndex =

_pages.length - pages.length; // Set to the first of the newly added pages.

if we're checking pages isn't empty above, we can feel confident page is not empty and update this.

TahaTesser · 2024-04-19T19:58:44Z

lib/src/modal_page/sliver_wolt_modal_sheet_page.dart

+  /// navigating to a specific page or popping a page.
+  final Object? id;
+
+


Suggested change

ulusoyca · 2024-04-28T20:41:24Z

Closing this because a new PR will be opened.

Add pop and push methods

9deda1e

ulusoyca mentioned this pull request Apr 19, 2024

Expose mainContentSlivers's enclosing context #176

Closed

Add imperative navigation methods

d2b02bb

ulusoyca force-pushed the introduce-imperative-navigation branch from 14ee6fe to d2b02bb Compare April 19, 2024 16:44

TahaTesser requested changes Apr 19, 2024

View reviewed changes

ulusoyca closed this Apr 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Breaking Change] Refactor page navigation in page list, remove pageIndexNotifier and pageListBuilderNotifier #182

[Breaking Change] Refactor page navigation in page list, remove pageIndexNotifier and pageListBuilderNotifier #182

ulusoyca commented Apr 19, 2024

github-actions bot commented Apr 19, 2024 •

edited

Loading

TahaTesser left a comment

TahaTesser Apr 19, 2024

TahaTesser Apr 19, 2024

TahaTesser Apr 19, 2024

TahaTesser Apr 19, 2024

TahaTesser Apr 19, 2024

TahaTesser Apr 19, 2024

TahaTesser Apr 19, 2024

TahaTesser Apr 19, 2024

TahaTesser Apr 19, 2024

TahaTesser Apr 19, 2024

ulusoyca commented Apr 28, 2024

	WoltModalSheetState? wms;
	WoltModalSheetState? woltModalSheetState;

	wms = context.state as WoltModalSheetState;
	woltModalSheetState = context.state as WoltModalSheetState;

	wms ??= context.findAncestorStateOfType<WoltModalSheetState>();
	woltModalSheetState ??= context.findAncestorStateOfType<WoltModalSheetState>();

		void pushPages(List<SliverWoltModalSheetPage> pages) {
		setState(() {

		/// navigating to a specific page or popping a page.
		final Object? id;

[Breaking Change] Refactor page navigation in page list, remove pageIndexNotifier and pageListBuilderNotifier #182

[Breaking Change] Refactor page navigation in page list, remove pageIndexNotifier and pageListBuilderNotifier #182

Conversation

ulusoyca commented Apr 19, 2024

Overview

Key Changes

Migration Notes

Testing

Future Work

Breaking Change

github-actions bot commented Apr 19, 2024 • edited Loading

TahaTesser left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ulusoyca commented Apr 28, 2024

github-actions bot commented Apr 19, 2024 •

edited

Loading