Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Go back to without popping each page #3048

Conversation

niimima
Copy link
Contributor

@niimima niimima commented Jan 14, 2024

Description of Change

Adding GoBackAsync methods without popping each page and NavigateFromAsync methods without recreating a page.

Bugs Fixed

API Changes

Added:

  • Task INavigationServiceExtensions.GoBackAsync(string viewName);
  • Task INavigationService.GoBackAsync(string viewName, INavigationParameters parameters);

Changed:

  • INavigationServiceExtensions.Task GoBackAsync(this INavigationService navigationService)

Behavioral Changes

No changes. Only additions of features. We have added casts due to errors in existing extension methods, but there are no changes in behavior.

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard

@niimima
Copy link
Contributor Author

niimima commented Jan 14, 2024

@dansiegel
I am in the process of implementing the API according to this comment: #2818 (comment)

As far as debugging goes, it seems to be working reasonably well. However, I am uncertain about the possible use cases and whether the behavior is truly correct. Additionally, I am unsure about the variations I should create in unit tests and struggling with deciding which class to target for unit tests (it should be the PageNavigationServiceFixture class, but it is not currently the subject of unit testing).

I apologize for reaching out amidst your busy schedule, but I would appreciate any advice you can provide.

Copy link
Member

@dansiegel dansiegel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be sure to add Unit tests for the new APIs. It would also be good to update the E2E app for manual testing and validation.

src/Maui/Prism.Maui/Navigation/INavigationService.cs Outdated Show resolved Hide resolved
src/Maui/Prism.Maui/Navigation/INavigationService.cs Outdated Show resolved Hide resolved
@dansiegel
Copy link
Member

Also would suggest you break this up into 2 PR's one for GoBack and one for NavigateFrom

@niimima
Copy link
Contributor Author

niimima commented Jan 15, 2024

Thank you for your advice.
This is my first pull request involving a feature change, so it might take a bit of time, but I'll give it a try. This PR will be focused on the GoBack.

@niimima
Copy link
Contributor Author

niimima commented Feb 11, 2024

@dansiegel 
Implemented end-to-end test: ba06722 , also implemented unit tests: a6e14b5.

@dansiegel dansiegel merged commit 17f270b into PrismLibrary:master Feb 11, 2024
6 of 7 checks passed
@dansiegel
Copy link
Member

Thanks for the PR @niimima

@niimima niimima changed the title [WIP] Go back to without popping each page Go back to without popping each page Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GoBackTo without popping each page
2 participants