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

[regression/8.0.0-preview.3.8149] Header in flyout menu makes first menu item not clickable on IOS #17965

Closed
michaelonz opened this issue Oct 12, 2023 · 29 comments · Fixed by #19125
Assignees
Labels
area-controls-shell Shell Navigation, Routes, Tabs, Flyout fixed-in-8.0.7 fixed-in-8.0.10 fixed-in-8.0.14 fixed-in-8.0.40 fixed-in-9.0.100-preview.1.9973 i/regression This issue described a confirmed regression on a currently supported version p/1 Work that is critical for the release, but we could probably ship without platform/iOS 🍎 t/bug Something isn't working
Milestone

Comments

@michaelonz
Copy link

michaelonz commented Oct 12, 2023

Description

When you have a shell flyout menu and have no Shell.FlyoutHeader then all the menu items work correctly on both ios and android.

If you add a Shell.FlyoutHeader to the flyout menu then the first flyoutmenu item on IOS is not clickable (android works fine).
NOTE: If you remove the Shell.FlyoutHeader then it works fine.

In the example shell.xaml below the first flyoutmenu item named 'Clicking Me Doesnt Work' wont work on IOS.

<Shell
    x:Class="DEMO.AppShell"
    xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
    xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
    xmlns:pages="clr-namespace:DEMO.Pages"
    Shell.FlyoutBehavior="Flyout">

    <Shell.Resources>
        <ResourceDictionary>
            <Style x:Key="BaseStyle" TargetType="Element">
                <Setter Property="Shell.BackgroundColor" Value="{StaticResource Primary}" />
                <Setter Property="Shell.ForegroundColor" Value="White" />
                <Setter Property="Shell.TitleColor" Value="White" />
                <Setter Property="Shell.DisabledColor" Value="#B4FFFFFF" />
                <Setter Property="Shell.UnselectedColor" Value="{AppThemeBinding Dark=#95FFFFFF, Light=#95000000}" />
                <Setter Property="Shell.TabBarBackgroundColor" Value="{AppThemeBinding Dark={StaticResource DarkBackground}, Light={StaticResource LightBackground}}" />
                <Setter Property="Shell.TabBarForegroundColor" Value="{AppThemeBinding Dark={StaticResource LightBackground}, Light={StaticResource DarkBackground}}" />
                <Setter Property="Shell.TabBarUnselectedColor" Value="{AppThemeBinding Dark=#95FFFFFF, Light=#95000000}" />
                <Setter Property="Shell.TabBarTitleColor" Value="{AppThemeBinding Dark={StaticResource LightBackground}, Light={StaticResource DarkBackground}}" />
            </Style>
            <Style BasedOn="{StaticResource BaseStyle}" TargetType="ShellItem" ApplyToDerivedTypes="True" />
        </ResourceDictionary>
    </Shell.Resources>
    <Shell.FlyoutHeader>
        
            <Grid HorizontalOptions="Fill" VerticalOptions="Start" HeightRequest="250" InputTransparent="True">
                <Image InputTransparent="True"
                           Aspect="AspectFill"
                           Source="demoperson.jpg"/>
                     <Grid RowDefinitions="40"
                  ColumnDefinitions="*"
                  RowSpacing="8"
                  HorizontalOptions="Fill"
                VerticalOptions="FillAndExpand"
                  Padding="15"
                   >
                <VerticalStackLayout  Grid.Row="0">
                <Label Text="Demo" FontSize="60" FontAttributes="Bold" VerticalOptions="Center" HorizontalOptions="FillAndExpand" HorizontalTextAlignment="Center" TextColor="White"></Label>
                <Label  Text="Adding life to people" FontSize="20" FontAttributes="Bold" VerticalOptions="Center" HorizontalOptions="FillAndExpand" HorizontalTextAlignment="Center" TextColor="White"></Label>
                </VerticalStackLayout>
            </Grid>
          </Grid>
        
    </Shell.FlyoutHeader>
 
  
    
    <FlyoutItem Title="Clicking Me Doesnt Work"
                FlyoutIcon="{StaticResource IconBluetoothLE}">
        <ShellContent ContentTemplate="{DataTemplate pages:HomePage}"
                      Route="HomePage" >
        </ShellContent>
    </FlyoutItem>

    <FlyoutItem Title="Session"
                FlyoutIcon="{StaticResource IconHeartRate}">
        <ShellContent ContentTemplate="{DataTemplate pages:HeartRatePage}"
                      Route="HeartRatePage">
        </ShellContent>
    </FlyoutItem>

    <FlyoutItem Title="User Management" 
                FlyoutIcon="{StaticResource IconUser}">
        <ShellContent ContentTemplate="{DataTemplate pages:EndUserList}"
                      Route="EndUserList" >
        </ShellContent>
    </FlyoutItem>

    <FlyoutItem Title="Settings"
                FlyoutIcon="{StaticResource IconGears}">
        <ShellContent ContentTemplate="{DataTemplate pages:SettingsPage}"
                      Route="SettingsPage">
        </ShellContent>
    </FlyoutItem>



    <Shell.FlyoutFooter>
        <VerticalStackLayout>
            <Label Text="© DEMO 2023" FontSize="Small"  VerticalOptions="Center" HorizontalOptions="Center" HorizontalTextAlignment="Center" TextColor="Black"></Label>
        </VerticalStackLayout>
    </Shell.FlyoutFooter>
    
</Shell>

Steps to Reproduce

  1. create new maui app
  2. use the Appshell.xaml from above to simulate issue)
  3. Run the app and on IOS the first menu item is not clickable

Link to public reproduction project repository

No response

Version with bug

8.0.0-preview.3.8149

Is this a regression from previous behavior?

Yes, this used to work in .NET MAUI

Last version that worked well

8.0.0-preview.2.7871

Affected platforms

iOS

Affected platform versions

16.6

Did you find any workaround?

#17965 (comment)

Relevant log output

No response

@michaelonz michaelonz added the t/bug Something isn't working label Oct 12, 2023
@samhouts samhouts added platform/iOS 🍎 area-controls-shell Shell Navigation, Routes, Tabs, Flyout potential-regression This issue described a possible regression on a currently supported version., verification pending labels Oct 12, 2023
@samhouts samhouts added this to the .NET 8 GA milestone Oct 12, 2023
@samhouts
Copy link
Member

Is this something that worked in 7.0 and is failing since updating to 8.0? Thanks!

@samhouts samhouts added the s/needs-info Issue needs more info from the author label Oct 12, 2023
@ghost
Copy link

ghost commented Oct 12, 2023

Hi @michaelonz. We have added the "s/needs-info" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@michaelonz
Copy link
Author

Hi - it worked in 7 - it’s a regression.

@ghost ghost added s/needs-attention Issue has more information and needs another look and removed s/needs-info Issue needs more info from the author labels Oct 12, 2023
@samhouts samhouts changed the title Header in flyout menu makes first menu item not clickable on IOS [regression/8.0.0] Header in flyout menu makes first menu item not clickable on IOS Oct 14, 2023
@jcsnider
Copy link

jcsnider commented Oct 14, 2023

Can confirm that this is an issue in .Net 8 and not .Net 7. Will update if I find any causes/workarounds.

@michaelonz
Copy link
Author

This is an issue in .net 8 rc2 and rc1

@jcsnider
Copy link

I just tested every .Net 8 preview until I found one with a different behavior...

In 8.0.0-preview.1.7762 and 8.0.0-preview.2.7871 there was a spacing issue between the header and the flyout menu items (ref #11679). There was a large gap between the header and the items but the items were all able to be interacted with. That visual spacing issue was fixed by PureWeen in #12480 and merged into 8.0.0-preview.3.8149.

Since 8.0.0-preview.3.8149 this issue has existed where the items are placed properly but the area that we can tap/interact with still seems to be blocked. I am not sure if related to #12480 but I think it's likely. Furthermore, It's not always the first item that can't be tapped, it can be the first and second based on device screen size and item size.

@michaelonz
Copy link
Author

Thanks @jcsnider for narrowing it down - will this be resolved in the next release?

@jcsnider
Copy link

jcsnider commented Oct 15, 2023

I'm not part of the team myself, and although I hope this is fixed in the next release I am proceeding as if it won't be and looking to work around it. Provided that this has been an issue for many months and not more popular I feel like it's due to something we are doing differently from the majority of users.

If I find any more info or workarounds I'll continue to update here. I'm also going to tag #17432 because I think these issues are related or the same.

Not sure if it helps but my Flyout header is very similar to yours with a grid that has a static height request with a background image? and a stack layout inside...

    <Shell.FlyoutHeader>
        <Grid
            x:Name="FlyoutHeaderGrid"
            BackgroundColor="Black"
            HeightRequest="200">

            <Image
                x:Name="FlyoutHeaderImage"
                Aspect="AspectFill"
                Opacity="1" />


            <VerticalStackLayout
                Margin="0,-20,0,0"
                HeightRequest="120"
                HorizontalOptions="Center"
                VerticalOptions="Center"
                WidthRequest="120">

                <Frame
                    x:Name="ProfileBackground"
                    Padding="0"
                    BackgroundColor="DarkGray"
                    CornerRadius="60"
                    HasShadow="False"
                    HeightRequest="120"
                    IsClippedToBounds="True"
                    WidthRequest="120">

                    <Image
                        x:Name="ProfilePicture"
                        HorizontalOptions="Center"
                        VerticalOptions="Center">
                        <Image.Source>
                            <FontImageSource
                                FontAutoScalingEnabled="False"
                                FontFamily="Font Awesome v6.3.0 Solid"
                                Glyph=""
                                Size="75"
                                Color="White" />
                        </Image.Source>

                    </Image>
                </Frame>

            </VerticalStackLayout>


            <Label
                x:Name="NameLabel"
                Margin="0,150,0,0"
                FontAttributes="Bold"
                FontAutoScalingEnabled="False"
                FontSize="22"
                HorizontalTextAlignment="Center"
                Text="Not Signed In"
                TextColor="White"
                VerticalTextAlignment="Center" />
        </Grid>
    </Shell.FlyoutHeader>

@michaelonz
Copy link
Author

michaelonz commented Oct 15, 2023

Hi @jcsnider , the only work around I’ve found was to have no menu items and then create the menu items using buttons and images in the header - so it looks the same but there is effectively no menu items.

This however is a terrible work around but if anyone is desperate to work around it then it does work - but means menus are unusable with a header right now in Maui 8

@jcsnider
Copy link

jcsnider commented Oct 15, 2023

@michaelonz -- I love it.. so simplistic and elegant. No idea why I didn't think of that. I'm gonna mess with this for another hour or so and then I'll fallback to that. Thanks.

Edit: I did find that if you give your element inside the header a Margin property value it will ruin the rendering of your header but the menu items will display in the correct spots and work -_-

@jcsnider
Copy link

So another option.. is to add a margin property to the top level view inside the header (in our case the grid) and also set a minimumheightrequest property that is a bit bigger than the heightrequest property on ios.

I added the following in my AppShell constructor.

            //TODO - Remove when https://github.com/dotnet/maui/issues/17965 is fixed
#if IOS
            FlyoutHeaderGrid.Margin = new Thickness(0, 0, 0, 0);
            FlyoutHeaderGrid.MinimumHeightRequest = FlyoutHeaderGrid.HeightRequest + 56;
#endif

The above makes my flyout work nearly perfectly. Afterwards the only issue the white horizontal bar above my header.. maybe a ios safe area thing? Either way I can live with it until a proper fix is released. Tested and working on an iPhone 8 simulator and a iPhone 14 Pro Max (very different sizes).

image

@michaelonz
Copy link
Author

@jcsnider - I will give this a try tomorrow also and report back my findings …..:)

@michaelonz
Copy link
Author

HI @jcsnider - Good work - that has resolved my issue also so it appears the issue is related to either one or both of the properties you have provided.

Maui Team still need to resolve it as its a tough one to narrow down - but that gets us past it for now (although i would like to try it on ipad etc etc)

@samhouts samhouts added i/regression This issue described a confirmed regression on a currently supported version and removed potential-regression This issue described a possible regression on a currently supported version., verification pending labels Oct 16, 2023
@samhouts
Copy link
Member

I just tested every .Net 8 preview until I found one with a different behavior...

In 8.0.0-preview.1.7762 and 8.0.0-preview.2.7871 there was a spacing issue between the header and the flyout menu items (ref #11679). There was a large gap between the header and the items but the items were all able to be interacted with. That visual spacing issue was fixed by PureWeen in #12480 and merged into 8.0.0-preview.3.8149.

Since 8.0.0-preview.3.8149 this issue has existed where the items are placed properly but the area that we can tap/interact with still seems to be blocked. I am not sure if related to #12480 but I think it's likely. Furthermore, It's not always the first item that can't be tapped, it can be the first and second based on device screen size and item size.

You're awesome!! Thank you so much for doing this research. We'll get started on the fix and plan to have it ready by GA or SR1, depending on the risk of the change.

@samhouts samhouts modified the milestones: .NET 8 GA, .NET 8 SR1 Oct 16, 2023
@samhouts samhouts removed the s/needs-attention Issue has more information and needs another look label Oct 16, 2023
@samhouts samhouts changed the title [regression/8.0.0] Header in flyout menu makes first menu item not clickable on IOS [regression/8.0.0-preview.3.8149] Header in flyout menu makes first menu item not clickable on IOS Oct 16, 2023
@mattleibow
Copy link
Member

Probably here in the comments, but looking closer and making item heights all different sizes I see that the first item is partially clickable if it is larger that the safe area height.

@biozal
Copy link

biozal commented Nov 30, 2023

I'm experiencing this issue also:

https://github.com/biozal/dotnet-cblite-inventory/blob/main/src/Dotnet.Cblite.Inventory.Maui/AppShell.xaml

iOS Specifically the first menu item isn't clickable. This is on an iPhone 13 Mini simulator specifically.

@AndyG74
Copy link

AndyG74 commented Dec 4, 2023

the issue already exists, but here i have found a workaround:

image

image

emaf added a commit that referenced this issue Dec 5, 2023
The Flyout Header height was being defined as `ArrangedHeaderViewHeightWithNoMargin` + `HeaderTopMargin`, but the `HeaderView.MeasuredHeight` (value on which `ArrangedHeaderViewHeightWithNoMargin` is based on) does include the margin so we were accounting for it twice and making the header go over the items below it, making them not clickable. When there is no margin, `HeaderTopMargin` represents the safe area and it should not be accounted for either.

Fixes #17965
kenancasey added a commit to FHU/Priorities that referenced this issue Dec 7, 2023
emaf added a commit that referenced this issue Dec 7, 2023
The Flyout Header height was being defined as `ArrangedHeaderViewHeightWithNoMargin` + `HeaderTopMargin`, but the `HeaderView.MeasuredHeight` (value on which `ArrangedHeaderViewHeightWithNoMargin` is based on) does include the margin so we were accounting for it twice and making the header go over the items below it, making them not clickable. When there is no margin, `HeaderTopMargin` represents the safe area and it should not be accounted for either.

Fixes #17965
@samhouts samhouts added the p/1 Work that is critical for the release, but we could probably ship without label Dec 7, 2023
@plppp2001
Copy link

I just confirmed this; If you add a Shell.FlyoutHeader to the flyout menu then the first flyoutmenu item on IOS is not clickable (android works fine).
NOTE: If you remove the Shell.FlyoutHeader then it works fine.

I didn't know this was an issue until I found this BUG report. Please fix.

@plppp2001
Copy link

I'm experiencing this issue also:

https://github.com/biozal/dotnet-cblite-inventory/blob/main/src/Dotnet.Cblite.Inventory.Maui/AppShell.xaml

iOS Specifically the first menu item isn't clickable. This is on an iPhone 13 Mini simulator specifically.

Yeah I just tested this, this is an issue for me developer friend.

@trampster
Copy link

This also occurs with FlyoutContent, the top 48 dp of FlyoutContent is not clickable if you have a FlyoutHeader

emaf added a commit that referenced this issue Dec 12, 2023
The Flyout Header height was being defined as `ArrangedHeaderViewHeightWithNoMargin` + `HeaderTopMargin`, but the `HeaderView.MeasuredHeight` (value on which `ArrangedHeaderViewHeightWithNoMargin` is based on) does include the margin so we were accounting for it twice and making the header go over the items below it, making them not clickable. When there is no margin, `HeaderTopMargin` represents the safe area and it should not be accounted for either.

Fixes #17965
emaf added a commit that referenced this issue Dec 12, 2023
The Flyout Header height was being defined as `ArrangedHeaderViewHeightWithNoMargin` + `HeaderTopMargin`, but the `HeaderView.MeasuredHeight` (value on which `ArrangedHeaderViewHeightWithNoMargin` is based on) does include the margin so we were accounting for it twice and making the header go over the items below it, making them not clickable. When there is no margin, `HeaderTopMargin` represents the safe area and it should not be accounted for either.

Fixes #17965
emaf added a commit that referenced this issue Dec 15, 2023
The Flyout Header height was being defined as `ArrangedHeaderViewHeightWithNoMargin` + `HeaderTopMargin`, but the `HeaderView.MeasuredHeight` (value on which `ArrangedHeaderViewHeightWithNoMargin` is based on) does include the margin so we were accounting for it twice and making the header go over the items below it, making them not clickable. When there is no margin, `HeaderTopMargin` represents the safe area and it should not be accounted for either.

Fixes #17965
emaf added a commit that referenced this issue Dec 20, 2023
The Flyout Header height was being defined as `ArrangedHeaderViewHeightWithNoMargin` + `HeaderTopMargin`, but the `HeaderView.MeasuredHeight` (value on which `ArrangedHeaderViewHeightWithNoMargin` is based on) does include the margin so we were accounting for it twice and making the header go over the items below it, making them not clickable. When there is no margin, `HeaderTopMargin` represents the safe area and it should not be accounted for either.

Fixes #17965
emaf added a commit that referenced this issue Dec 22, 2023
The Flyout Header height was being defined as `ArrangedHeaderViewHeightWithNoMargin` + `HeaderTopMargin`, but the `HeaderView.MeasuredHeight` (value on which `ArrangedHeaderViewHeightWithNoMargin` is based on) does include the margin so we were accounting for it twice and making the header go over the items below it, making them not clickable. When there is no margin, `HeaderTopMargin` represents the safe area and it should not be accounted for either.

Fixes #17965
emaf added a commit that referenced this issue Dec 22, 2023
The Flyout Header height was being defined as `ArrangedHeaderViewHeightWithNoMargin` + `HeaderTopMargin`, but the `HeaderView.MeasuredHeight` (value on which `ArrangedHeaderViewHeightWithNoMargin` is based on) does include the margin so we were accounting for it twice and making the header go over the items below it, making them not clickable. When there is no margin, `HeaderTopMargin` represents the safe area and it should not be accounted for either.

Fixes #17965
@michaelonz
Copy link
Author

@emaf it looks like you have done a lot of commits to resolve this - can you give any indication when this might ship? I want to know if i need to start programming work arounds of if the next release will have a resolution and rough ball park release date so we can plan internally (I know you cant give a fixed date but an indication of a target would be appreciated)

github-actions bot pushed a commit that referenced this issue Jan 8, 2024
The Flyout Header height was being defined as `ArrangedHeaderViewHeightWithNoMargin` + `HeaderTopMargin`, but the `HeaderView.MeasuredHeight` (value on which `ArrangedHeaderViewHeightWithNoMargin` is based on) does include the margin so we were accounting for it twice and making the header go over the items below it, making them not clickable. When there is no margin, `HeaderTopMargin` represents the safe area and it should not be accounted for either.

Fixes #17965
@emaf
Copy link
Contributor

emaf commented Jan 8, 2024

@michaelonz if everything goes alright, the plan is to ship those changes in .NET 8 SR2. We need to land those changes in main first, and then see how they go before being certain about it.

Once the changes landed in main, you can use nightly builds to give those a try.

PureWeen pushed a commit that referenced this issue Jan 9, 2024
* [iOS] Fix Flyout Header breaking clicking over items

The Flyout Header height was being defined as `ArrangedHeaderViewHeightWithNoMargin` + `HeaderTopMargin`, but the `HeaderView.MeasuredHeight` (value on which `ArrangedHeaderViewHeightWithNoMargin` is based on) does include the margin so we were accounting for it twice and making the header go over the items below it, making them not clickable. When there is no margin, `HeaderTopMargin` represents the safe area and it should not be accounted for either.

Fixes #17965

* Rename confusing properties

Rename `ArrangedHeaderViewHeightWithNoMargin` and `MeasuredHeaderViewHeightWithNoMargin`, to `ArrangedHeaderViewHeightWithMargin` and `MeasuredHeaderViewHeightWithMargin`, since both include the top margin value: https://github.com/dotnet/maui/blob/main/src/Core/src/Layouts/LayoutExtensions.cs#L27.

* Fix tests

* Fix safe area margin bottom value

* Fix margins

* Polish flyout header and content offsets

* Remove extra space

* Simplify content layout

* Small code cleanup

* Fix condition

* Simplify test

* More tests

* Test fix

* Add ArrangedHeaderViewHeightWithOutMargin

* Broken

* Fix scrollview inset

* Fix all tests

* Fix scroll view content inset and expand tests

* Fix new test cases for Android

* Skip FlyoutHeaderContentAndFooterAllMeasureCorrectly for Android since there is a bug that makes tests fail

* Enable more tests for iOS

* Honor IgnoreSafeArea

* Add missing null check

* Fix CollectionView support

* Add check by ItemsView for CV

* Add tests scrolling using different content control types

* Log more info on new tests

* Use requested value for assertion

* Consider epsilon for scrolled position

* Better test logging

* Fix spacing

* Another indenting issue

* Fix namespaces
@bcaceiro
Copy link

This isn't fixed in the latest release. Still can't click when having an header...

@mattleibow
Copy link
Member

Can you try the nightly builds? I see this was merged 2 weeks ago, but due to other reasons the sr1 that shipped a few days ago was actually supposed to go out a few weeks ago.

So effectively this was merged after the release branch was created. This should be in the next release.

@plppp2001
Copy link

plppp2001 commented Jan 20, 2024 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-shell Shell Navigation, Routes, Tabs, Flyout fixed-in-8.0.7 fixed-in-8.0.10 fixed-in-8.0.14 fixed-in-8.0.40 fixed-in-9.0.100-preview.1.9973 i/regression This issue described a confirmed regression on a currently supported version p/1 Work that is critical for the release, but we could probably ship without platform/iOS 🍎 t/bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.