microsoft / microsoft-ui-xaml

Windows UI Library: the latest Windows 10 native controls and Fluent styles for your applications
MIT License
6.35k stars 677 forks source link

Proposal: Improve context menu support for listviews #911

Open vgromfeld opened 5 years ago

vgromfeld commented 5 years ago

Proposal: Improve context menu support for listviews

Summary

Adding context menus to list view items is more complicated than it should be. Depending on where we define the menu, it can be complicated to get a contextual menu working with command binding, accelerators, and all the available input types.

Rationale

My goal was to add the same contextual menu to all my list view's items. Today, we have several options to do that:

Define a ContextFlyout in our items' DataTemplate

<DataTemplate x:Key="itemTemplateWithLocalContextMenu">
  <Grid Background="Transparent">
    [...]
    <Grid.ContextFlyout>
      <MenuFlyout>
        <MenuFlyoutItem Command="{Binding Command}" CommandParameter="{Binding}" DataContext="{Binding}" Text="Click me from item template">
          <MenuFlyoutItem.KeyboardAccelerators>
            <KeyboardAccelerator Key="A" />
          </MenuFlyoutItem.KeyboardAccelerators>
        </MenuFlyoutItem>
      </MenuFlyout>
    </Grid.ContextFlyout>
  </Grid>
</DataTemplate>

The binding is working without any effort since we are within the data template and have access to the item But:

Define a ContextFlyout on a custom ListViewItem style

<ListView.ItemContainerStyle>
    <Style TargetType="ListViewItem">
        <Setter Property="ContextFlyout">
            <Setter.Value>
                <MenuFlyout>
                    <MenuFlyoutItem Command="{Binding Command}" CommandParameter="{Binding}" DataContext="{Binding}" Text="Click me from ListViewItem">
                        <MenuFlyoutItem.KeyboardAccelerators>
                            <KeyboardAccelerator Key="B" />
                        </MenuFlyoutItem.KeyboardAccelerators>
                    </MenuFlyoutItem>
                </MenuFlyout>
            </Setter.Value>
        </Setter>
    </Style>
</ListView.ItemContainerStyle>

With this approach, the ContextFlyout is now opening fine when right clicking anywhere on the ListViewItem. The keyboard context menu key is also working. But:

Define a ContextFlyout at the ListView level

<ListView
    ItemTemplate="{StaticResource itemTemplate}"
    ItemsSource="{x:Bind Items}">
    <ListView.ContextFlyout>
        <MenuFlyout>
            <MenuFlyoutItem
                Command="{x:Bind ((local:DataItem)listViewWithContextFlyout.SelectedItem).Command, Mode=OneWay}"
                CommandParameter="{x:Bind listViewWithContextFlyout.SelectedItem, Mode=OneWay}"
                Text="Click me from ListView">
                <MenuFlyoutItem.KeyboardAccelerators>
                    <KeyboardAccelerator Key="C" />
                </MenuFlyoutItem.KeyboardAccelerators>
            </MenuFlyoutItem>
        </MenuFlyout>
    </ListView.ContextFlyout>
</ListView>

This is working fine more most of the scenario but:

Define a ContextFlyout on a custom ListViewItem style and use XAMLUICommand

This option allow us to move the command registration outside of the ListViewItems but it still required some code to get the current/selected item from the ListView. It does not solve the missing ListViewItem.DataContext/Content inside the context flyout. XAMLUICommand is only available starting with RS5 which make it not usable when targeting a lower Windows build.

Write custom code

Using some custom code, we can get a fully working ContextFlyout but it requires to force the list view item selection when right clicking on it, do some code to set the DataContext on the ContextFlyout content to make the binding work.

Scope

Capability Priority
Defining a per item contextual menu should be straightforward Must
Binding should work in a ContextFlyout Must
Keyboard accelerator should "just" work executing the command on the current item Must

Important Notes

The definition of a per item context flyout should be easy and MVVM compliant without any need for custom code to have everything working (mouse, keyboard, accelerators).

Sample application highlighting the different behaviors and attempts.

ContextMenu.zip ContextMenu2.zip

lukasf commented 5 years ago

I guess a lot of devs know this problem and have coded their own workarounds. A solution in WinUI would be very appreciated.

My thoughts on this:

When the ListView is in multi selection mode, and you right-click on one of the selected items, you'd probably want the menu command to apply to all selected items. This is what Windows does everywhere.

So I'd recommend to define this as a new property on the ListView itself, e.g. ListView.ItemsMenuFlyout. The CommandParameter would always be a list of items. When right-clicking on a selected item, the list would contain all selected items. When right-clicking on an unselected item, the list would only contain the clicked item.

Note: This requires the Command to be defined on the top-level ViewModel, not on the individual items.

mdtauk commented 5 years ago

There would have to be a way for some items to either disable, add or remove items from the Context Menu Flyout on a per item basis.

YuliKl commented 4 years ago

Making a note to also consider #1487 when designing this feature.

r-laf commented 3 years ago

Regarding Define a ContextFlyout on a custom ListViewItem style, it's also possible to get the DataModel like this:

        private SampleDataModel GetDataModelForCurrentListViewFlyout()
        {
            // Obtain the ListViewItem for which the user requested a context menu.
            var listViewItem = SharedFlyout.Target;

            // Get the data model for the ListViewItem.
            return (SampleDataModel)ItemListView.ItemFromContainer(listViewItem);
        }

From here.

omikhailov commented 3 years ago

It would be easier to make a new ListView rather than fix the old one. It has a lot of issues, for example, the element does not know whether it is selected or not, the first double click is swallowed and list just changes selection instead, Reveal styles and animations are enabled by default despite that new shadow effects would be often more appropriate, and so on. It has so many oddities that it will be difficult to maintain compatibility.

JaiganeshKumaran commented 2 years ago

@vgromfeld This inside the data template for the list view:

<Grid Background="Transparent">
    [...]
    <Grid.ContextFlyout>
      <MenuFlyout>
        <MenuFlyoutItem Command="{Binding Command}" CommandParameter="{Binding}" DataContext="{Binding}" Text="Click me from item template">
          <MenuFlyoutItem.KeyboardAccelerators>
            <KeyboardAccelerator Key="A" />
          </MenuFlyoutItem.KeyboardAccelerators>
        </MenuFlyoutItem>
      </MenuFlyout>
    </Grid.ContextFlyout>
  </Grid>

Won't work well since the grid != list view item. It's inside the list view item when loaded however usually there's padding between the content inside and the item so the menu will show up only within the bounds of the grid. If you used a control that's focusable instead of grid, you should be able to use the keyboard context menu button.

Aurumaker72 commented 2 years ago

This issue has stagnated and situation still hasn't improved. More modern Stackoverflow answers recommend equally counterintuitive anti-patterns.