xamarin / XamarinCommunityToolkit

The Xamarin Community Toolkit is a collection of Animations, Behaviors, Converters, and Effects for mobile development with Xamarin.Forms. It simplifies and demonstrates common developer tasks building iOS, Android, and UWP apps with Xamarin.Forms.
MIT License
1.59k stars 474 forks source link

[Bug] Expander doesn't work properly inside Shell Flyout on iOS #570

Open AdamDiament opened 4 years ago

AdamDiament commented 4 years ago

Hi @AndreiMisiukevich thanks for your work creating the expander control, which is a great addition to xamarin forms that I've been hoping for for a long time. I've found an iOS bug which I've detailed below, I wondered if you could take a look or if you know of any workarounds? Funnily enough I coded a bespoke expander just for my project before this was available which always suffered from this but, and in the process of replacing mine with your new control I found that it suffered from the same behaviour on iOS - so it's possible our implementations were similar - I could never work out why it was happening - I hope you can! Thanks!

Description

When using the new experimental Expander as the content for a shell item template in a Shell Flyout item on iOS, the expansion doesn't push down the other Flyout content items as expected. Instead it overlays the other content, and clicks go straight through to the content below it. Android works as expected, the other content moves down out of the way, clicks in the expanded zone are captured as expected, and clicking the expander header again hides the items and the other content moves back up.

Steps to Reproduce

  1. Launch the repro project I have provided in iOS
  2. Open the flyout menu
  3. Click on the "Click me to expand" link at the top of the screen
  4. You will see the revealed content overlaying the other flyout items, the other items don't move down out of the way.
  5. Try to click on one of the revealed buttons. None of the buttons are clickable (except the first one with some effort). Most of the time the clicks fall through to the underlying flyout items.

Expected Behavior

The other flyout content should move down out of the way to reveal the expandable content, and move back up when the content is hidden by clicking on the expandable header again. The expandable content items should be clickable if they are supposed to be e.g buttons. Here is a short gif of the app running on my physical android device (samsung galaxy a4 android 9.0), showing it displaying the correct behaviour.

Android - expected behaviour

Actual Behavior

On iOS the expanded content comes over the top of the other items and clicks sink through to the other flyout items below it. Here is a short gif of the app running on my iPhone XS (13.3.1) showing the unexpected wrong behaviour.

iOS - unexpected behaviour

If the orientation is changed to landscape and back to portrait while the expander is open, the other items are redrawn in the correct place. However closing the expander does not redraw them, unless the orientation changes are repeated once closed.

Basic Information

Screenshots

See gifs above

Reproduction Link

Here's a zipped reproduction solution with Android and iOS projects. I've removed all bin and obj folders, so make sure to rebuild.

ShellWithExpanderBugTest.zip

Workaround

None found - even if I include my expander at the bottom of the flyout to avoid the "other content not getting out of the way" issue, the revealed buttons are still not clickable. For now on iOS I will need to have a different menu system.

AndreiMisiukevich commented 4 years ago

Hi @AdamDiament Thank you for the excellent report.

Since it works fine on Android, I presume we should try to find the root cause in iOS-specific Shell code.

AdamDiament commented 4 years ago

Thanks for the quick reply @AndreiMisiukevich. Yes I think you are right about where to look - I'd love to help and I will definitely have a look, although in all honesty I think it may be a bit beyond my expertise.

@PureWeen (oh master of all things shell-based :-) )- I wonder if you fancy another tasty shell bug after you fixed that shell navigation one I found last month, that got added in 4.6

pictos commented 3 years ago

@AdamDiament can you confirm if this issue still happens?

Keflon commented 3 years ago

I have a similar issue that is not related to Shell (I'm not using Shell) that looks like it may have the same root cause, so I'm sharing my investigations in the hope it will help the team find and fix it. ...

Environment: XF 4.8.0.1821. VS, PC, Mac, Xcode all up to date at time of writing. iOS only. Android works as expected.

Cause: For a ContentPage containing an Expander, the Expander Content becomes invisible to user input if the following are true:

  1. The ContentPage is within a NavigationPage. (Don't know if this is important)
  2. The NavigationPage is the Detail property of a Flyout.
  3. The user uses the Flyout UI to swap to a different Detail page, and then back again.

Underlying problem: My guess is that gestures are not propagating from the Expander to it's Content properly after the Expander has been removed and re-added to the visual tree.

Notes: The Expander Header receives gestures at all times. The problem is only for the Expander Content.

Attempted workarounds (none helped)

  1. Every time the Expander is Expanded, I called ForceLayout on the ContentPage.

  2. In OnPageAppearing I set Expander.Content to null and then back to the original content.

  3. This is my Expander:

    <Expander x:Name="TheExpander" IsExpanded="{Binding IsExpanded}" VerticalOptions="Fill" BackgroundColor="Transparent">
    <Expander.Header>
        Blah ...
    </Expander.Header>
    <StackLayout x:Name="TheMenu"
        BindableLayout.ItemsSource="{Binding Options}"
        BindableLayout.ItemTemplate="{StaticResource DropdownOptionTemplate}" />
    </StackLayout>
    </Expander>

    I wrote a custom StackLayout that inflates the DropdownOptionTemplate itself, so I could monitor the resultant UI for every bound item: a) InputTransparent is not being set. b) References to the ICommand implementations (in my ViewModel) are valid on the appropriate Controls. c) The underlying Command instances are not becoming disabled. d) The BindingContext of the inflated UI is not being overwritten.

  4. I put the StackLayout directly into the ContentPage to confirm it works as expected. (It does).

  5. I tried rebuilding the ItemsSource in my ViewModel every time the Expander is expanded, and let the Binding rebuild the UI.

    • I did this by replacing the Options instance outright, by Clear()ing the existing Options and re-adding new items, and by Removing items one-by-one then adding replacements back one-by-one.
  6. I hooked the TapGestureRecognizer Tapped event to code-behind. It fails when the ICommands fail.

  7. Tried a ListView instead of a StackLayout.

I am using a MasterDetailPage (XF 4.8.0.1821 so no Flyout rename yet).

I need to ship, so my next two things to try are:

  1. Write my own Expander, in the hope it's Expander related rather than something horrible in the layout engine
  2. Update to XF 5 and hope it's fixed and does not cause further problems.

I'll try both. If anyone has further information (root cause / workaround etc) I'd really appreciate any guidance.

PureWeen commented 3 years ago

@AdamDiament is this still happening on 5.0? @pictos and I did a good amount of work to make sure Flyout Items respond to size changes 🤞

Keflon commented 3 years ago

Hi @pureween. I perhaps should have said I think my issue may be related because AdamDiament noted that

'... and clicks sink through to the other flyout items below it.'

and

'... the revealed buttons are still not clickable.'.

Both statements are consistent with my Expander Content becoming transparent with regard to input.

Additional info:

  1. If my Expander is 'expanded' when I swap my Detail page out and back, the Expander continues to work as expected and I cannot break it by collapsing it and swapping Detail pages in and out, so I guess something horrid is happening to the Content of the Expander if the Content is not visible when the Expander is first removed from the visual tree.

  2. I wrote a replacement Expander and it has exactly the same behaviour, though sometimes it doesn't break, and if it doesn't initially break I cannot then force it to break during the same app instance.

  3. I have the same problem with XF5.

If this is not relevant here please let me know if I should report it somewhere else.

AdamDiament commented 3 years ago

@AdamDiament is this still happening on 5.0? @pictos and I did a good amount of work to make sure Flyout Items respond to size changes 🤞

@PureWeen @pictos Thanks for looking into this. I'm struggling to get hot restart working to test this. It might be something to do with the account holder at our apple developer account needing to accept the latest licenses. Hopefully I'll be able to test later this week.

AdamDiament commented 3 years ago

@PureWeen @pictos i managed to get an iOS simulator up and running. There has definitely been some improvement, as the expander opens and closes and moves the other items out the way now, and the individual items can be touched. However, the expander click seems to "fall through" to the "close fly out" method. I am setting FlyoutIsPresented = true in the click handler to override this so you can see the expander expand/contract. Is there a way to get the fly out never to close at all when clicking the expander open / close?

https://user-images.githubusercontent.com/6513834/105645090-786d3f00-5e67-11eb-897a-a5bfd5dec188.mp4

AdamDiament commented 3 years ago

@PureWeen @pictos @AndreiMisiukevich

An update for this, the behaviour where expanding or closing the expander on iOS closes the flyout unexpectedly, that I mentioned in my comment from 25th Jan, still happens on XF 5.0.0.2012. I have got a workaround for this, it's a bit ugly but it works ok-ish.

  1. Remove the expander.header entirely
  2. In a grid, have a stack layout containing the ui from the expander header, and on top covering it in the z stack, a transparent button with no text.
  3. In the button click handler, programmatically open / close the expander.

This way the flyout stays open when expanding. On iOS the header does disappear and reappear with every expansion/contraction with this workaround, but it's better than closing the flyout:

Like this ios:

https://user-images.githubusercontent.com/6513834/112802853-5cf6fe00-907b-11eb-9932-b2fef0b17df9.mp4

And it remains working fine on android with the workaround

https://user-images.githubusercontent.com/6513834/112802881-641e0c00-907b-11eb-8bde-9d356be55698.mp4

Workaround XAML:

 <Shell.ItemTemplate>
            <DataTemplate>
                <StackLayout>
                    <!--This grid is a workaround to make the expanding menu work on iOS.
                    If and when the bug is fixed, it can be done away with and the stack layout can be moved back to the Expander.Header-->

                    <Grid RowDefinitions="35">
                        <StackLayout
                            Orientation="Horizontal">
                                           **THE UI PREVIOUSLY IN THE EXPANDER.HEADER, LABEL / CHEVRON ETC**
                        </StackLayout>
                        <Button BackgroundColor="Transparent" Clicked="Button_OnClicked"></Button>
                    </Grid>

                    <toolkit:Expander x:Name="Expander">
                    <!--<toolkit:Expander.Header>
                            **Where the header ui should go in an ideal circumstance, if iOS wasn't being weird.**
                    </toolkit:Expander.Header>-->
                    <StackLayout Margin="60,0,0,0">
                              **... menu items, to be displayed when expanded**
                    </StackLayout>
                </toolkit:Expander>
                </StackLayout>
            </DataTemplate>
        </Shell.ItemTemplate>

Click handler


    private async void Button_OnClicked(object sender, EventArgs e)
    {
      var tapped = sender as VisualElement;
      var expander = tapped?.Parent.FindByName<Expander>("Expander");

      if (expander != null)
      {
        expander.IsExpanded = !expander.IsExpanded;

        var element = tapped.Parent.FindByName<Label>("Chevron");
        element.CancelAnimations();
        await element.RotateXTo(flipped ? 0 : 180, 190, Easing.Linear);
        flipped = !flipped;
      }
    }
pictos commented 3 years ago

@PureWeen taking in consideration the workaround, this is still a bug or this is the expected way that devs should implement it?

VikyaSurve commented 3 years ago

@PureWeen @pictos @AndreiMisiukevich

An update for this, the behaviour where expanding or closing the expander on iOS closes the flyout unexpectedly, that I mentioned in my comment from 25th Jan, still happens on XF 5.0.0.2012. I have got a workaround for this, it's a bit ugly but it works ok-ish.

  1. Remove the expander.header entirely
  2. In a grid, have a stack layout containing the ui from the expander header, and on top covering it in the z stack, a transparent button with no text.
  3. In the button click handler, programmatically open / close the expander.

This way the flyout stays open when expanding. On iOS the header does disappear and reappear with every expansion/contraction with this workaround, but it's better than closing the flyout:

Like this ios:

iosexpander.mp4 And it remains working fine on android with the workaround

androidexpander.mp4 Workaround XAML:

 <Shell.ItemTemplate>
            <DataTemplate>
                <StackLayout>
                    <!--This grid is a workaround to make the expanding menu work on iOS.
                    If and when the bug is fixed, it can be done away with and the stack layout can be moved back to the Expander.Header-->

                    <Grid RowDefinitions="35">
                        <StackLayout
                            Orientation="Horizontal">
                                           **THE UI PREVIOUSLY IN THE EXPANDER.HEADER, LABEL / CHEVRON ETC**
                        </StackLayout>
                        <Button BackgroundColor="Transparent" Clicked="Button_OnClicked"></Button>
                    </Grid>

                    <toolkit:Expander x:Name="Expander">
                    <!--<toolkit:Expander.Header>
                            **Where the header ui should go in an ideal circumstance, if iOS wasn't being weird.**
                    </toolkit:Expander.Header>-->
                    <StackLayout Margin="60,0,0,0">
                              **... menu items, to be displayed when expanded**
                    </StackLayout>
                </toolkit:Expander>
                </StackLayout>
            </DataTemplate>
        </Shell.ItemTemplate>

Click handler


    private async void Button_OnClicked(object sender, EventArgs e)
    {
      var tapped = sender as VisualElement;
      var expander = tapped?.Parent.FindByName<Expander>("Expander");

      if (expander != null)
      {
        expander.IsExpanded = !expander.IsExpanded;

        var element = tapped.Parent.FindByName<Label>("Chevron");
        element.CancelAnimations();
        await element.RotateXTo(flipped ? 0 : 180, 190, Easing.Linear);
        flipped = !flipped;
      }
    }

Can you please share a sample project for this?

AdamDiament commented 3 years ago

@VikyaSurve hi there. I don't have the time to add a repro project right now but my reproduction steps are pretty comprehensive, if you want to have a look. Thanks!

VikyaSurve commented 3 years ago

@VikyaSurve hi there. I don't have the time to add a repro project right now but my reproduction steps are pretty comprehensive, if you want to have a look. Thanks!

I am very new for this, if you can share the repos in day or two. it can be very helpful

AdamDiament commented 3 years ago

@VikyaSurve In fact I looked back at my original comment from May 2020 and I already provided a reproduction project, It might be out of date now but you can use it as a start if you like.

Reproduction Link

Here's a zipped reproduction solution with Android and iOS projects. I've removed all bin and obj folders, so make sure to rebuild.

ShellWithExpanderBugTest.zip

fexxdev commented 2 years ago

Hi! I have the same exact problem as yours @AdamDiament. Is there any update to this? Thanks

lantona commented 1 year ago

I have the exact problem. is there an update?