MaterialDesignInXAML / MaterialDesignInXamlToolkit

Google's Material Design in XAML & WPF, for C# & VB.Net.
http://materialdesigninxaml.net
MIT License
15.23k stars 3.44k forks source link

Binding issues with ContextMenu and CompositeCollection #3464

Open michaelmairegger opened 9 months ago

michaelmairegger commented 9 months ago

Bug explanation

I have XAML Binding Failures with MenusAndToolBars.xaml if I change the lines https://github.com/MaterialDesignInXAML/MaterialDesignInXamlToolkit/blob/master/MainDemo.Wpf/MenusAndToolBars.xaml#L355-L360 with the follwogin content:

<ContextMenu>
  <ContextMenu.ItemsSource>
    <CompositeCollection>
      <MenuItem Header="Hello World" />
      <MenuItem Header="Clickety Click">
        <MenuItem Header="Clackety Clack" />
      </MenuItem>
    </CompositeCollection>
  </ContextMenu.ItemsSource>
</ContextMenu>

saying:

Severity Count Data Context Binding Path Target Target Type Description File Line Project Error 2 null (0) MenuItem.Height Double Cannot find source: RelativeSource FindAncestor, AncestorType='System.Windows.Controls.Primitives.MenuBase', AncestorLevel='1'.

Do you have an idea how to fix this issue?

Version

4.9.0

nicolaihenriksen commented 9 months ago

@michaelmairegger Is there a particular reason you choose the ContextMenu.ItemsSource property instead of just the XAML below?

The ItemsSource property is often used when the objects you have are not UI elements, and a DataTemplate is the used to visualize these "data" items. In your case, you are putting actual UI elements into the collection. I suspect this is also the reason for the binding error. Without having verified my theory, I would assume that the MenuItem instances you put in the CompositeCollection are not (at least not initially) part of the visual tree, but only the logical tree, and the binding which attempts to travel up the visual tree to find the root ContextMenu (i.e. MenuBase) then ends up failing.

<ContextMenu>
  <MenuItem Header="Hello World" />
  <MenuItem Header="Clickety Click">
    <MenuItem Header="Clackety Clack" />
  </MenuItem>
</ContextMenu>

or

<ContextMenu>
  <ContextMenu.Items>
      <MenuItem Header="Hello World" />
      <MenuItem Header="Clickety Click">
        <MenuItem Header="Clackety Clack" />
      </MenuItem>
  </ContextMenu.Items>
</ContextMenu>
michaelmairegger commented 9 months ago

@nicolaihenriksen Yes there is a reason. I have a "BaseMenu" that contains multiple MenuItems that are reused within many views, e.g.

<UserControl.Resources>
  <CompositeCollection x:Key="BaseMenu">
    <MenuItem Header="Add" />
    <MenuItem Header="Edit" />
    <MenuItem Header="Delete" />
  </CompositeCollection>
</UserControl.Resources>

<ContextMenu>
  <ContextMenu.ItemsSource>
    <CompositeCollection >
      <CollectionContainer Collection="{StaticResource BaseMenu}" />
      <Separator />
      <MenuItem Header="Hello World" />
      <MenuItem Header="Clickety Click">
        <MenuItem Header="Clackety Clack" />
      </MenuItem>
    </CompositeCollection>
  </ContextMenu.ItemsSource>
</ContextMenu>
binarycow commented 9 months ago

@michaelmairegger - try this:

<ContextMenu>
  <ContextMenu.ItemsSource>
    <CompositeCollection >
      <CollectionContainer Collection="{Binding Source={StaticResource BaseMenu}}" /> <!-- Note the binding -->
      <Separator />
      <MenuItem Header="Hello World" />
      <MenuItem Header="Clickety Click">
        <MenuItem Header="Clackety Clack" />
      </MenuItem>
    </CompositeCollection>
  </ContextMenu.ItemsSource>
</ContextMenu>

See here: https://stackoverflow.com/a/40147645

michaelmairegger commented 8 months ago

Unfortunately it is not working. I created a workaround by modifying the style and removing the two lines that created the issue

MichelMichels commented 8 months ago

Unfortunately it is not working. I created a workaround by modifying the style and removing the two lines that created the issue

Could you tell us which lines?

michaelmairegger commented 8 months ago

I have removed the lines https://github.com/MaterialDesignInXAML/MaterialDesignInXamlToolkit/blob/master/MaterialDesignThemes.Wpf/Themes/MaterialDesignTheme.Menu.xaml#L230 and https://github.com/MaterialDesignInXAML/MaterialDesignInXamlToolkit/blob/master/MaterialDesignThemes.Wpf/Themes/MaterialDesignTheme.Menu.xaml#L236

I have found no unexpected visual behavior in my application that got introduced by removing those two lines

MichelMichels commented 8 months ago

Thank you very much for your reply. I'll try to investigate this further.

MichelMichels commented 8 months ago

@michaelmairegger I can't replicate the behavior you're reporting. I added this XAML code in the demo app in search of replicating this:

<smtx:XamlDisplay Margin="0,0,16,16" UniqueKey="menus_6">
  <smtx:XamlDisplay.Resources>
    <CompositeCollection x:Key="BaseMenu">
      <MenuItem Header="Add" />
      <MenuItem Header="Edit" />
      <MenuItem Header="Delete" />
    </CompositeCollection>
  </smtx:XamlDisplay.Resources>

  <TextBox Width="256" Text="With CompositeCollection (see issue #3464)">
    <TextBox.ContextMenu>
      <ContextMenu>
        <ContextMenu.ItemsSource>
          <CompositeCollection>
            <CollectionContainer Collection="{StaticResource BaseMenu}" />
            <Separator />
            <MenuItem Header="Hello World" />
            <MenuItem Header="Clickety Click">
              <MenuItem Header="Clackety Clack" />
            </MenuItem>
          </CompositeCollection>
        </ContextMenu.ItemsSource>
      </ContextMenu>
    </TextBox.ContextMenu>
  </TextBox>
</smtx:XamlDisplay>

Disclaimer: I'm using the master branch to test this bug.

michaelmairegger commented 8 months ago

Ok thanks for the information. I will try it on my end, and add a branch to my fork that replicates the issue.

michaelmairegger commented 8 months ago

I have added your code to MenuAndToolBars.xaml and the following gif shows the behavior

Issue 3464

MichelMichels commented 8 months ago

Apologies, my mistake, I was expecting a more blocking runtime exception. Now, I see the same problem you are seeing. I'll look into what the effect is of removing those lines you mentioned.

MichelMichels commented 8 months ago

This only seems to be an issue when used with a ContextMenu, a regular Menu does not give this error. I'm not sure why at this moment.

UPDATE: I think it has to do with the AdornerDecorator in the ControlTemplate of the default style for context menu. Will test tomorrow.

michaelmairegger commented 8 months ago

Thanks. I think the main issue will be that CompositeCollection is not part of the visual tree

MichelMichels commented 8 months ago

Yes, this indeed seems to be the case. The AdornerDecorator is not the problem. I tested this.

I don't know if we will be able to resolve this as the default style for MenuItem is expecting a MenuBase usercontrol as its parent somewhere in the tree. I'm trying to get myself familiar with CompositeCollection, but its the first time that I work with this class.

michaelmairegger commented 8 months ago
<UserControl.Resources>
  <CompositeCollection x:Key="BaseMenu">
    <MenuItem Header="Add" />
    <MenuItem Header="Edit" />
    <MenuItem Header="Delete" />
  </CompositeCollection>
</UserControl.Resources>

<ContextMenu>
  <ContextMenu.ItemsSource>
    <CompositeCollection >
      <CollectionContainer Collection="{StaticResource BaseMenu}" />
      <Separator />
      <MenuItem Header="Hello World" />
      <MenuItem Header="Clickety Click">
        <MenuItem Header="Clackety Clack" />
      </MenuItem>
    </CompositeCollection>
  </ContextMenu.ItemsSource>
</ContextMenu>

I came across this issue because of the situation that I do not want to define the same menu structure over and over again. Therefore I have created a Base menu

MichelMichels commented 8 months ago

Yes, I understand what you are trying to do. My last comment was explaining why this is happening.

There might be some confusion about MenuBase:

To be clear, this explanation above is not the issue.

The issue is, as you mentioned, that CompositeCollection is not a part of the Visual Tree. That is why following XAML line defined in the MaterialDesignMenuItem style is failing:

<Setter Property="Height" Value="{Binding Path=(wpf:MenuAssist.TopLevelMenuItemHeight), RelativeSource={RelativeSource Mode=FindAncestor, AncestorType=MenuBase}}" />

This line expects there to be a UserControl of class MenuBase to be in the Ancestors of the MenuItem; which is not the case due to the CompositeCollection.

michaelmairegger commented 8 months ago

This line expects there to be a UserControl of .... due to the CompositeCollection.

Yes, unfortunately this is true

There might be some confusion about MenuBase: No, it was clear that my BaseMenu as ResourceKey has nothing to do with x:Type MenuBase. I just wantet to state why I have such a situation