Skip to content

Fix little bug, return original page. also Add cancel button feature for UWP#354

Closed
huangjinshe wants to merge 2 commits intoRedth:masterfrom
huangjinshe:master
Closed

Fix little bug, return original page. also Add cancel button feature for UWP#354
huangjinshe wants to merge 2 commits intoRedth:masterfrom
huangjinshe:master

Conversation

@huangjinshe
Copy link
Copy Markdown

Fix exception, fix return page problem. and add show cancel button
feature.

1.Fix when finish scan stop, the mediaCapture.stopPreview() will execute more than onece, then there is a excpetion problem.
2.Old UWP will always use the current mainWindow Frame, that cause a lot of problem, like can't return to the original page. So I create a new Frame and replace mainWindow.Content, after finish scan just restore the original mainWindow.Content. All fine, I test it on real phone.

Now you could remove the description about add on MainWindow on Home page, because you don't need it anymore:
NavigationCacheMode="Enabled"

3.I add a cancel button. But If you want to use it, just need to set the CancelButtonText. like:

var scanner = new ZXing.Mobile.MobileBarcodeScanner(); scanner.CancelButtonText = "Return";

That will more helpful because you don't need to press Back button on the phone, and cause some trouble.

Fix exception, fix return page problem. and add show cancel button
feature.
Also add cancel button feature to UWP
@f1nzer f1nzer mentioned this pull request Jul 4, 2016
@Redth
Copy link
Copy Markdown
Owner

Redth commented Jul 18, 2016

@f1nzer I'm curious what your thoughts are on this idea for UWP (creating a new frame).... I do like the idea of being able to remove the NavigationCacheMode attribute...

@huangjinshe
Copy link
Copy Markdown
Author

huangjinshe commented Jul 18, 2016

@Redth NavigationCacheMode not working if your using Xamarin MasterDetailPage.
Also I change the main frame will avoid much more problem I guess, don't need to change some default setting.

I also add return cancel button. If still use old NavigationCacheMode, that will not working in Xamarin as I said before. it will let the Xamarin main navigation mess up.

@huangjinshe
Copy link
Copy Markdown
Author

I'm test it based on the Xamarin+UWP. So I just need some one could test it on UWP project.
Because if MainWindow is different than current page, I worry that will cause the problem. Or I will take a time to check it myself.

@huangjinshe
Copy link
Copy Markdown
Author

I just create a UWP project and check, I see some code in OnLaunched():

 Frame rootFrame = Window.Current.Content as Frame;
 Window.Current.Content = rootFrame;

So if UWP also use this way, I think it's same as Xamarin. I guess there is no problem.
I thought it's different than Xamarin, more like WPF. now I'm not worry about it.

@huangjinshe
Copy link
Copy Markdown
Author

also I really don't know how you guys test about NavigationCacheMode="Enabled", it doesn't work after I test it before.

@Redth Redth mentioned this pull request Jul 18, 2016
@Redth Redth added this to the 2.2.0 milestone Jul 18, 2016
@f1nzer
Copy link
Copy Markdown
Contributor

f1nzer commented Jul 19, 2016

As for me it's very common practice to use NavigationCacheMode in UWP apps and it's not the only place where you should use this property in your app (tombstombing is great!). For now I don't know how to implement it's in a better way (mby to use ContentDialog? 😄).

Creating new frame will cause many problems such as:

  • back button handling usually implemented globally via SystemNavigationManager.GetForCurrentView().BackRequested += handler and passing new frame will break this logic so we should handle it's somehow
  • source app may use custom frame and our new default frame will break it again

This PR doesn't handle first point in any way so app closes on hardware back button click.
Also can you give me small sample with MasterDetailPage implementation and sample when NavigationCacheMode doesn't work for you? We can check what can we do in this case.

@huangjinshe
Copy link
Copy Markdown
Author

huangjinshe commented Jul 19, 2016

@f1nzer Actually I've already uploaded the sample about MasterDetailPage problem.
#350

Go and download this sample.
As you see, you could found if you press back button on the phone, it will close the app. which mean NavigationCacheMode not let the navigation return. (looks like not only master detail page, navigation page also can't press back button to return back)

Another problem which the page mess up after scan back when navigate. I didn't reproduced today.(maybe that problem will reproduce when click the back button which I added, not the real back button on the phone. Maybe that's why I change the original logic to new Frame)

Let's see the original code:

var rootFrame = RootFrame ?? Window.Current.Content as Frame ?? ((FrameworkElement) Window.Current.Content).GetFirstChildOfType<Frame>();
This is the original logic, and that will use the original Frame, that will cause too much problem. when the begin I want to add cancel button only, after I add it, when I go back, it will change my original navigation page, and I need to go navigate the page again, also there is some problem when I navigate. I check the code and see:

if (!isNewInstance && Frame.CanGoBack)
                Frame.GoBack();

That's why I change it to new frame, new frame don't worry change the original frame. a separate part. Don't need to worry too much.

Now let's face on those problems your mention:

  1. Why you consider the first problem now? not before? Because in the original code it's also using the frame, it also can't cancel when press back button on the phone. So I suggest we should recommend the user use the back button in the screen, not the back button. at least we had a time to think and let user use the feature for now. Also maybe there is no real back button in the future on windows phone (like iPhone and Android), on the desktop application also no back button on the navigation, because it's the first page.
  2. I don't know why you ask like this. Why the original code use source frame all fine, but new frame will broke the source frame? I think that's like a opposite point. Currently new frame no any deal with source frame, but the original code use source frame and that will cause problems. Our new default frame no any relationship with source frame, So I guess that's more good than the original way.

@f1nzer
Copy link
Copy Markdown
Contributor

f1nzer commented Jul 19, 2016

@huangjinshe NavigationCacheMode is used to preserve page state and does not affect on your navigation. For example you need to fill login, password and scan barcode: to preserve login and password fields you should use NavigationCacheMode (if you want to see your fields filled after scanning).

In your MasterDetailsPage sample you just need to add handler for hardware back button (#362).

Why you consider the first problem now? not before? Because in the original code it's also using the frame, it also can't cancel when press back button on the phone. So I suggest we should recommend the user use the back button in the screen, not the back button. at least we had a time to think and let user use the feature for now.

You must enable back navigation for all hardware and software system back buttons. (check this link https://msdn.microsoft.com/en-us/library/windows/apps/mt465734.aspx). So it's not our problem if it's not enabled (we can just remind users don't forget this in readme.md).

I don't know why you ask like this. Why the original code use source frame all fine, but new frame will broke the source frame? I think that's like a opposite point. Currently new frame no any deal with source frame, but the original code use source frame and that will cause problems. Our new default frame no any relationship with source frame, So I guess that's more good than the original way.

You can also check "default" suggested implementation for back button:

Frame rootFrame = Window.Current.Content as Frame;

If anyone uses this logic => you can expect app closing with your new frame logic because backstack on your frame is empty and event is not handled.
Or also users may use custom frame (defined in a field for example). In this case if u press back button during scanning user's back logic will pop previous page from the frame and your frame will be ignored (because we rely only on our own frame and have no idea that there are may be another frames).

@huangjinshe
Copy link
Copy Markdown
Author

huangjinshe commented Jul 19, 2016

Ok. I got you, I hope you guys could add this in this project home page, avoid developers still couldn't return to original page.

SystemNavigationManager.GetForCurrentView().BackRequested += (sender, e) =>
{
   Frame rootFrame = Window.Current.Content as Frame;
   if (!e.Handled && rootFrame != null && rootFrame.CanGoBack)
   {
      e.Handled = true;
      rootFrame.GoBack();
   }
}

Also If you guys want to keep cancel button feature, please tell developers: in the default the cancel button will show on (because you guys give it the default value already : "Cancel" ). If developers want to hide it, just remove the value from it:
scanner.CancelButtonText = "";

Also please keep the bug fix about the mediaCapture.stopPreview() will execute more than once.,

@f1nzer
Copy link
Copy Markdown
Contributor

f1nzer commented Jul 19, 2016

I have some ideas about back button implementation and NavigationCacheMode notification.
Also I've found the problem with cancel button you've mentioned here.
I'll make separate PR later today.

@f1nzer f1nzer mentioned this pull request Jul 20, 2016
@huangjinshe
Copy link
Copy Markdown
Author

I close it.

NavigationCacheMode="Enabled" and SytemNavigationManager.GetForCurrentView().BackRequested += handler in main page solution is good. I agree you said :"You must enable back navigation for all hardware and software system back buttons"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants