builttoroam / BuildIt

Build It is the set of libraries developed by Built to Roam that makes it easier and quicker to build applications
MIT License
34 stars 0 forks source link

ContentButton in Xamarin Forms sends command after scrolling #116

Open kanadaj opened 5 years ago

kanadaj commented 5 years ago

As said in the title, embedding the ContentButton in a ScrollView and then scrolling the view causes the button to move with the user's finger, so the command is sent on release.

nickrandolph commented 5 years ago

@kanadaj for clarification can you break your explanation into expected behaviour and actual behaviour, just so I'm clear on what the issue is.

kanadaj commented 5 years ago

Sure! The expected behaviour would be that if you drag the scroll view containing the Content Button control while holding the button, the button shouldn't detect a tap event since the scroll view should take priority.

Actual behaviour is that the Content Button detects a tap (possibly a long tap) and fires the bound Command while trying to scroll. I must also add that this happens even if I don't release the button, it executes the Command after a second or so even if I never release it.

kanadaj commented 5 years ago

After some testing, it seems to me that the TouchEffect's TouchAction is never called with TouchActionType.Exited nor with TouchActionType.Entered

kanadaj commented 5 years ago

Note, in the end I ended up replacing the ContentButton with a custom control using https://github.com/mrxten/XamEffects and a fairly simple:

<ContentView.ControlTemplate>
  <ControlTemplate>
      <Grid xamEffects:TouchEffect.Color="#66000000" xamEffects:Commands.Tap="{TemplateBinding InternalCommand}" xamEffects:Commands.TapParameter="{TemplateBinding CommandParameter}">
          <ContentPresenter Content="{TemplateBinding Content}"/>
          <BoxView Color="Transparent"/>
          <!--<Button BackgroundColor="Transparent" Command="{TemplateBinding Command}" CommandParameter="{TemplateBinding CommandParameter}" Clicked="Button_OnClicked" Padding="0" Margin="0" WidthRequest="0" HorizontalOptions="Fill" VerticalOptions="Fill"/>-->
      </Grid>
  </ControlTemplate>
</ContentView.ControlTemplate>

with InternalCommand being a simple command calling

private void ExecuteInternalCommand()
{
    if (Command != null && Command.CanExecute(CommandParameter))
    {
        Command.Execute(CommandParameter);
    }
    else
    {
        Pressed?.SafeRaise(this, null);
    }
}

The press effect is much better (it uses the native "wave" effect) and the behaviour is far more consistent than the original ContentButton here. Maybe it'd be worth considering to replace the implementation here with it?

nickrandolph commented 5 years ago

Yeh we'd considered removing the ContentButton for exactly this reason - good find and thanks for posting feedback here.