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

Fix destory older pages when navigation from NavigationPage #1051

Closed
wants to merge 65 commits into from
Closed

Fix destory older pages when navigation from NavigationPage #1051

wants to merge 65 commits into from

Conversation

nuitsjp
Copy link
Contributor

@nuitsjp nuitsjp commented May 17, 2017

Please take a moment to fill out the following:

Fixes issue #1045.

Changes proposed in this pull request:

When there are multiple pages in Navigation Stack, when navigation from NavigationPage, Destroy of removed page from Stack is not called.
Fixed this problem.

matheusneder and others added 14 commits May 4, 2017 10:00
…Of T that works for complex property. It takes the complex object to subscribe PropertyChanged listener as first parameter and a lambda with what property to observe as second parameter.
…Command Of T that works for complex property. It takes the complex object to subscribe PropertyChanged listener as first parameter and a lambda with what property to observe as second parameter."

This reverts commit 8afcf60.
If Detail is null, change to set the next Page to Detail in all cases.
…t does not make parts of Prism public API.
…etPropertyValue; Re-implemented PropertyObserverNode.GetPropertyValue by getting property's value by Reflection instead of Expression.Compile in order to avoid different behavior observed in Windows/Android related to iOS.
…l variable in PropertyObserverNode.GetPropertyValue
If Detail is NavigationPage, it is not necessary because it is handled correctly by ProcessNavigationForPage Method.
@nuitsjp
Copy link
Contributor Author

nuitsjp commented May 17, 2017

Completed. Please review.

@brianlagunas
Copy link
Member

brianlagunas commented May 25, 2017

Can you please rebase to fix the conflicts so I can start reviewing this PR :)

no longer swollow the exceptions made in the NavigateAsync method.  We log and rethrow.
@nuitsjp
Copy link
Contributor Author

nuitsjp commented May 25, 2017

OK, I will respond during a few days.
Probably it will be about 28 days.
I want to fix UnitTest a little.

brianlagunas and others added 4 commits May 25, 2017 16:10
[XF] NavigationService: no longer swallow exceptions
…peToViewModelTypeResolverWhenSet

After executing this UnitTest, all ViewModel becomes ViewModelLocatorFixture.
For this reason, other tests result in errors.
@nuitsjp
Copy link
Contributor Author

nuitsjp commented May 26, 2017

I tried Rebase, but I'm not sure if this is fine.
If there seems to be a problem, I would like you to tell me that it will be rebuilt.

I have another consultation.

I fixed UnitTest for ViewModelLocatorFixture#ShouldUseCustomDefaultViewTypeToViewModelTypeResolverWhenSet.
After executing this UnitTest, all ViewModel becomes ViewModelLocatorFixture.
For this reason, other tests result in errors.
But maybe not a very good test code.
Could you tell me if there is a better idea?

@brianlagunas
Copy link
Member

I don't think the re-base was done properly. It seems you pulled in all the changes instead of rebasing. Could you please rebase? Maybe it will be easier for you to close this PR and open a new one?

I am not sure I understand the issue with the test. Can you open a new issue so we can discuss the issue and discuss possible fixes?

Thanks for your great work!

@nuitsjp
Copy link
Contributor Author

nuitsjp commented May 27, 2017

OK. I make it again.
And the test code creates another Issue.

@nuitsjp
Copy link
Contributor Author

nuitsjp commented May 28, 2017

I made a new PR #1051 , so I will close it.

@nuitsjp nuitsjp closed this May 28, 2017
@nuitsjp
Copy link
Contributor Author

nuitsjp commented May 28, 2017

I was wrong.
Here is the correct PR.

@nuitsjp
Copy link
Contributor Author

nuitsjp commented May 28, 2017

I made a new PR #1051 , so I will close it.

@nuitsjp nuitsjp closed this May 28, 2017
@lock
Copy link

lock bot commented Jan 28, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants