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

Preventing inadvertant multiple views caused by double-taps in Prism.Forms navigation #442

Closed
craigblazakis opened this issue Feb 2, 2016 · 28 comments
Labels

Comments

@craigblazakis
Copy link

The attached zip file contains a small sample Xamarin Forms solution using Prism that illustrates a navigation issue that I am experiencing.

The PCL contains a navigation page and two content pages (Page1View and Page2View). The app is initialized with Page1View hosted in the navigation page.

this.NavigationService.Navigate("NavigationPage/Page1View");

Page1View contains a label with a TapGestureRecognizer that is bound to a command on it's view model that navigates to Page2View.

this.navigationService.Navigate("Page2View");

This works as expected. When the user clicks on the label on Page1View, the app navigates to Page2View, which continues to show the toolbar and a back button. Page2View also contains a label and TapGestureRecognizer that allow prism-based navigation back to Page1View.

The issue that I am experiencing is that the app allows the user to inadvertently tap on the label on Page1View multiple times, resulting in multiple instances of Page2View being displayed on top of each other.

I have not been able to find an easy way to prevent this behavior. Initially, I tried looking for ways to modify the TapGestureRecognizer so that it would not respond to double-taps or multiple sequential single-taps, but I was not able to find a solution.

My next attempt was to make use of the DelegateCommand's can-execute functionality. The approach I tried was to set the DelegateCommand's can-execute to false in the Action method, and then to set it back to true in the over-ridden OnNavigatedTo() method in the Page1ViewModel class after the user navigates back from Page2View. This works. Multiple taps no longer cause multiple views to be displayed. However, the approach fails when the user navigates back to Page1View using the back button on the toolbar because, in this case, the OnNavigatedTo() method is not called, and the can-execute is never set back to true.

Is there something wrong in my implementation? Or, am I overlooking a simple way to prevent this behavior? I am new to these technologies, so I would not be surprised to learn that I am missing something simple.

Thanks,
Craig

ButtonNavigation.zip

@craigblazakis craigblazakis changed the title Preventing inadvertant multiple views caued by double-taps in Prism.Forms navigation Preventing inadvertant multiple views caused by double-taps in Prism.Forms navigation Feb 2, 2016
@craigblazakis
Copy link
Author

A follow-up question related to my initial post...

Isn't it a problem that the INavigationAware methods are executed ONLY when the user navigates using the INavigationService, but NOT when they navigate using the toolbar back button? It seems to me that users may expect those methods the be called all of the time.

@brianlagunas
Copy link
Member

As you can probably guess, this is not an issue related to Prism, but rather the TapGestureRecognizer. I myself have run into this behavior. This was my workaround:

int _tapCount = 0;
private void Navigate()
{
    _tapCount += 1;
    if (_tapCount > 1)
    {
        _tapCount = 0;
        return;
    }                
    _navigationService.Navigate("Page2View");
}

Yes, I would expect the INavigationAware methods to be invoked when using the hardware and software buttons. The problem is that Xamarin.Forms does not expose an API for me to use in order to support that scenario. So until Xamarin adds an API for me to hook into, I can't do anything about it. The good news is that I am working with Xamarin to get something like this added. I don't know when they will get it in, but it is something we are discussing.

@craigblazakis
Copy link
Author

Incidentally, using a tap count to prevent multiple taps did not work for me. It does prevent multiple taps, but it seemed to have a side effect. When a navigation is permitted, the _tapCount is not set back to 0 and remains set to 1. This means that the very next tap after navigating back will not be permitted.

Instead, I am using a time interval approach which seems to work well.

    private readonly TimeSpan minTapInterval = new TimeSpan(0, 0, 2);  // 2 seconds
    private DateTime lastTapTimestamp;

    protected void Navigate(string name, NavigationParameters np)
    {
        var now = DateTime.Now;
        try
        {
            if (now - this.lastTapTimestamp < this.minTapInterval)
            {
                return;
            }

            this.navigationService.Navigate(name, np);
        }
        finally
        {
            this.lastTapTimestamp = now;
        }
    }

@brianlagunas
Copy link
Member

I'm glad you found a workaround. Now if we can just get Xamarin to add a property to their TapGesturerecognizer to have a MaxNumberOfTaps or similar to prevent more than one tap.

@dhaligas
Copy link

dhaligas commented Apr 1, 2016

@brianlagunas I am seeing this as well. Could this be an issue with the navigation service? perhaps a timing issue?

@brianlagunas
Copy link
Member

@dhaligas this is not an issue with the Navigation service. This is the result of a multiple button clicks that occur rapidly in succession. You could try setting the CanExecute of the command to false before Navigation and then set it back to true after navigation, or you can use a timer approach like @craigblazakis did in the code snippet above.

@brianlagunas
Copy link
Member

In the next release you'll be able to simply do this to prevent a double click:

        private bool _canNavigate = true;
        public bool CanNavigate
        {
            get { return _canNavigate; }
            set { SetProperty(ref _canNavigate, value); }
        }
NavigateCommand = new DelegateCommand(Navigate).ObservesCanExecute((vm) => CanNavigate);
        async void Navigate()
        {
            CanNavigate = false;
            await _navigationService.NavigateAsync("ViewB");
            CanNavigate = true;
        }

@dhaligas
Copy link

dhaligas commented Jun 1, 2016

@brianlagunas nice! when does the next release drop?

@brianlagunas
Copy link
Member

I will be releasing a new preview release soon. I am leaving for NDC Olso soon, so it might be a couple of weeks.

@craigblazakis
Copy link
Author

Hi Brian,

Any word on when a release that includes the features to prevent double-taps that you described above will be available?

Also, any word on when a release that includes INavigationAware functionality in response to navigation initiated from Android Toolbar buttons, such as the back button, might be available? I know this is something that you were working on.

Looking forward to the updates!

Thanks,
Craig

@brianlagunas
Copy link
Member

I may release another preview soon, but I can't release as stable until XF 2.3.1 is released.

Xamarin still hasn't exposed any API for me to tap into fo rthe hardware/software buttons.

@craigblazakis
Copy link
Author

Thanks for the update. I wish there were a way to motivate the Xamarin team to expose that API sooner rather than later!

@brianlagunas
Copy link
Member

I just released a preview 6 to NuGet that has this fix.

Yeah, me too. Unfortunately, the Xamarin.Forms team has been pretty unresponsive since Evolve.

@craigblazakis
Copy link
Author

They are Microsofties now!

Thanks for the release!! Really appreciate it.

@craigblazakis
Copy link
Author

Brian,

In the sample code that you listed above, you are using an "async void" method that does not return a Task. It seems that an alternative might be to return a Task and then use the DelegateCommand.FromAsyncHandler() method. I am inclined to use your code as is, however, because the Navigate() method is essentially acting as a UI event handler. Any thoughts?

@dvorn
Copy link
Contributor

dvorn commented Jun 28, 2016

@craigblazakis @brianlagunas Task-returning method is better. Because in tests you will be able to await on the command and then do assertions - after the command actually completed.

@dude3133
Copy link

dude3133 commented Jan 4, 2017

Actually it is not closed. NavigationService should handle this case.
For example you got 3 buttons which navigates to different views (viewA, viewB and viewC) if user will tap on second button, than in same moment on first and third it should navigate only on viewB, not on viewB->viewA->viewC

@brianlagunas
Copy link
Member

@dude3133 No, the navigation service should not be responsible for this. This is application logic. The navigation service is only responsible for navigating. That's it. Since you have decided to provided three buttons in a UI that all navigate, it is your responsibility to support your application behavior. You can easily do this using the CanExecute of your navigate commands. See above snippet.

@dude3133
Copy link

dude3133 commented Jan 5, 2017

@brianlagunas Okay, another example:
I got master details page and different viewmodels for master and details. There is button both on master and details. How should I know about clicking on button from different viewmodel? Why not create property in NavigationService like IsNavigating so I could know about navigating? Or I should create some crutch to manage this?

@brianlagunas
Copy link
Member

brianlagunas commented Jan 5, 2017

This is not a scenario I think you should be concerned about. While technically possible, this is not a scenario that is very common. If a user does do this, then they are placing that behavior on themselves as that is a very deliberate end-user action. I have never seen an app that would prevent navigation in one part of the screen based on the immediate action of a button press on another part of a screen's menu. Also keep in mind that each VM gets a different instance of a INavigationService. Due to the nature of dealing with an unknown number of object instances across an entire application and the async nature of navigation in XF, it would be very difficult to create a reliable mechanism to do this. Not to mention the fact that not all apps wish to behave this way. So you would have to have a mechanism to turn it off as well. Even XF navigation does not have this built in. This is just not something that Prism will ever do.

@hnabbasi
Copy link

hnabbasi commented Feb 3, 2017

@brianlagunas is correct. This is not a bug in NavigationService. If you send it 2 commands, it will execute 2 commands. You have to manage the locking/unlocking somewhere in your view model.

Unfortunately, the CanNavigate approach only works for navigation to pages and not to pop-ups. You can double-tap a button to present a pop-up and find out that you have two pop-ups displayed, maybe more.

To fix this, I have a TaskRunner service (kinda like Xamarin's Sports app but not a static class), that runs all my tasks. All my commands (to navigate or show a pop-up, etc) are tasks that are run via TaskRunner service. I lock the navigation with CanNavigate in my BaseViewModel before calling the TaskRunner.RunTask() and unlock it in two places. Once in the OnNavigatedTo virtual method in my BaseViewModel, and once in the AlertService after it displays the pop-up. This takes care of double/triple taps issue.

@tbaggett
Copy link

@hnabbasi do you mind providing your implementation to solve this? I also have popups to contend with and would love to use your approach. Thanks!

@brianlagunas
Copy link
Member

To prevent multiple button clicks: #442 (comment)

@hnabbasi
Copy link

Sure, my implementation is based on what Brian also suggested in comment #422. I have a blog post explaining it all here https://intelliabb.com/2017/02/18/handling-multiple-taps-in-xamarin-forms-on-android

@tbaggett
Copy link

Thanks, @brianlagunas, for the multiple button click fix reference. I was interested in reviewing @hnabbasi 's solution since it also takes popups into account.

Thanks to both of you for the quick response and links!

@chekodev
Copy link

`using System;
using System.Collections.Generic;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
using System.Text;

using Xamarin.Forms;

using Prism.Mvvm;
using Prism.Navigation;

namespace Test.ViewModels
{
public abstract class ViewModelBase : BindableBase, INavigationAware
{
#region Services

    protected readonly INavigationService navigationService = null;

    #endregion

    public ViewModelBase(
        INavigationService navigationService
        )
    {
        this.navigationService = navigationService;
    }

    #region Navigation

    public virtual void OnNavigatedFrom(NavigationParameters parameters) { isNavigating = false; }

    public virtual void OnNavigatedTo(NavigationParameters parameters) { }

    public virtual void OnNavigatingTo(NavigationParameters parameters) { }

    public virtual bool OnBackButtonPressed => true;

    public void BackButtonPressed()
    {
        if (navigationService != null)
            if (OnBackButtonPressed)
                navigationService.GoBackAsync();
    }

    private bool isNavigating;

    protected Task SafeNavigateAsync(string name, NavigationParameters parameters = null, bool? useModalNavigation = null, bool animated = true)
    {
        if (isNavigating)
            return Task.CompletedTask;
        isNavigating = true;
        try { return navigationService.NavigateAsync(name, parameters, useModalNavigation, animated); }
        catch { return Task.CompletedTask; }
    }

    #endregion

}

}
`

@olivertech
Copy link

Hi @brianlagunas I have implemented your suggestion with

NavigateCommand = new DelegateCommand(Navigate).ObservesCanExecute((vm) => CanNavigate);

And it is working, but I am getting a colateral effect that I would like to resolve...

I have a menu with a lot of buttons. When I tap any one, all buttons are disabled on the same time... for me it is not all a problem, but all button textcolor turns into a dark color that I do not like. I would like to maintain the textcolor, the same one in enabled state (which is white).

I wanna change this textcolor in disabled state, but I am not finding a way.

I have made a custom button, as below:

    public class CustomButton : Button
    {
        private Style normalStyle;

        public Style DisabledStyle
        {
            get { return (Style)GetValue(DisabledStyleProperty); }
            set { SetValue(DisabledStyleProperty, value); }
        }

        public static readonly BindableProperty DisabledStyleProperty = BindableProperty.Create("DisabledStyle", 
                                                                                            typeof(Style), 
                                                                                            typeof(CustomButton), 
                                                                                            null, 
                                                                                            BindingMode.TwoWay, 
                                                                                            null,
                                                                                            (obj, oldValue, newValue) => { });

        public CustomButton()
        {
            normalStyle = this.Style;
            this.PropertyChanged += CustomButton_PropertyChanged;
        }

        private void CustomButton_PropertyChanged(object sender, PropertyChangedEventArgs e)
        {
            if (e.PropertyName == "IsEnabled" && this.DisabledStyle != null)
            {
                if (this.IsEnabled)
                    this.Style = normalStyle;
                else
                    this.Style = DisabledStyle;
            }
        }
    }

And I Have this xaml code:

<custom:CustomButton x:Name="btnFaleConosco" DisabledStyle="{StaticResource ButtonDisabled}" HorizontalOptions="Start".... />

And here I defined the disabled style, inside my masterdetail page:

        <ContentPage.Resources>
            <ResourceDictionary>
                <Style x:Key="ButtonDisabled" TargetType="Button">
                    <Setter Property="TextColor" Value="White" />
                </Style>
            </ResourceDictionary>
        </ContentPage.Resources>

The custom event is called, but nothing happens.

So, is my code correct or is there any other way to change the disabled text color for all buttons ?

Thanks.

@lock
Copy link

lock bot commented Jan 29, 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 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

9 participants