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

NavigationView in 2.4 doesn't return the Item anymore in ItemInvoked #2520

Closed
michael-hawker opened this issue May 21, 2020 · 4 comments · Fixed by #2682
Closed

NavigationView in 2.4 doesn't return the Item anymore in ItemInvoked #2520

michael-hawker opened this issue May 21, 2020 · 4 comments · Fixed by #2682
Assignees
Labels
area-NavigationView NavView control team-Controls Issue for the Controls team
Milestone

Comments

@michael-hawker
Copy link
Collaborator

Describe the bug
The 2.4 release broke the ItemInvoked API contract by returning the realized UI Element from the new underlying ItemsRepeater instead of the Item from the underlying MenuItemsSource collection.

Steps to reproduce the bug

Sample Code
    <Page.Resources>
        <DataTemplate x:Key="CategoryTemplate"
                      x:DataType="local:SampleCategory">
            <Grid>
                <TextBlock VerticalAlignment="Center"
                           FontFamily="Segoe UI"
                           FontSize="15px"
                           FontWeight="Normal"
                           Text="{x:Bind Name}" />
            </Grid>
        </DataTemplate>
    </Page.Resources>

    <winui:NavigationView x:Name="NavView"
                          PaneDisplayMode="Top"
                          MenuItemTemplate="{StaticResource CategoryTemplate}"
                          ItemInvoked="NavView_ItemInvoked"
                          SelectionFollowsFocus="Disabled"
                          IsSettingsVisible="True"/>
Code Behind
        private List<SampleCategory> sampleCategories = new List<SampleCategory>()
        {
            new SampleCategory()
            {
                Name = "Controls",
                Samples = new Sample[] {
                    new Sample()
                    {
                        Name = "UniformGrid",
                        Subcategory = "Panels",
                        About = "Some Info...",
                    },
                    new Sample()
                    {
                        Name = "WrapPanel",
                        Subcategory = "Panels",
                        About = "Some Info...",
                    }
                }
            },
            new SampleCategory()
            {
                Name = "Extensions",
                Samples = new Sample[] {
                    new Sample()
                    {
                        Name = "Mouse",
                        About = "Some Info...",
                    },
                    new Sample()
                    {
                        Name = "OnDevice",
                        About = "Some Info...",
                    }
                }
            }
        };

        public MainPage()
        {
            this.InitializeComponent();

            NavView.MenuItemsSource = sampleCategories;
        }

Steps to reproduce the behavior:

  1. Open sample (or re-create from pasted code below)
  2. Run sample
  3. Click on 'Controls' item in NavigationView
  4. Should see message dialog from our working 2.3.x code (but don't)
  5. Note: that we don't get the same result between the previous package and 2.4, this breaks apps trying to upgrade!

See the 2.3.200213001 branch on the same repo. You can switch to that which has the previous NuGet package setup to see the difference in behavior.

Actual behavior
NavigationViewItemInvokedEventArgs.ItemInvoked is a UIElement of the realized data template.

Expected behavior
Should return the underlying Item in the args from the MenuItemsSource (as it did before in 2.3).

Screenshots

Item Invoked for Item (Controls):

  • 2.3.200213001
    image

  • 2.4.2
    image
    Note: Here we're not getting our item as we'd expect!

Item Invoked for Settings (this looks the same and seems fine, but adding for completeness)
  • 2.3.200213001
    image

  • 2.4.2
    image

Version Info

NuGet package version: Microsoft.UI.Xaml 2.4.2

Windows 10 version Saw the problem?
Insider Build (xxxxx)
May 2020 Update (19041) Yes
November 2019 Update (18363)
May 2019 Update (18362)
October 2018 Update (17763)
April 2018 Update (17134)
Fall Creators Update (16299)
Creators Update (15063)
Device form factor Saw the problem?
Desktop Yes
Mobile
Xbox
Surface Hub
IoT

Additional context

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label May 21, 2020
@ranjeshj ranjeshj added area-NavigationView NavView control needs-cherrypicktorelease PR tagged for cherry-pick to the current release branch (but not yet picked) team-Controls Issue for the Controls team labels May 21, 2020
@ranjeshj ranjeshj added this to the WinUI 2.5 milestone May 27, 2020
@ranjeshj
Copy link
Contributor

@ojhad can you investigate this issue ?

@ranjeshj ranjeshj removed the needs-triage Issue needs to be triaged by the area owners label May 27, 2020
@michael-hawker
Copy link
Collaborator Author

michael-hawker commented Jun 11, 2020

@ojhad @ranjeshj is this fixed in the new 2.5 prerelease package?

This is a regression and breaking change to anyone using this event on NavigationView updating to 2.4.x, so will it be patched in the 2.4 line as well?

@marcelwgn
Copy link
Contributor

I don't think there has been a fix for this yet.

@ojhad Are you looking into this right now or would you like me to look into this/mind me looking into this? CC @ranjeshj

@ojhad
Copy link
Contributor

ojhad commented Jun 12, 2020

Are you looking into this right now

Yep, I have a fix in the works. This is a symptom of a deeper underlying bug for this scenario.

@ranjeshj ranjeshj removed the needs-cherrypicktorelease PR tagged for cherry-pick to the current release branch (but not yet picked) label Aug 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-NavigationView NavView control team-Controls Issue for the Controls team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants