AvaloniaUI / Avalonia

Develop Desktop, Embedded, Mobile and WebAssembly apps with C# and XAML. The most popular .NET UI client technology
https://avaloniaui.net
MIT License
26.04k stars 2.25k forks source link

MenuItem HotKey only works after the menu item has been displayed at least once. #2441

Open Michron opened 5 years ago

Michron commented 5 years ago

It looks like this is the same issue which was fixed in #290. If the HotKey property of a MenuItem is set, it will not work until the user has opened the menu which contains that MenuItem.

To reproduce the bug simply add the following changes to the Menu sample in the ControlCatalog. In MenuItemViewModel.cs

    public class MenuItemViewModel
    {
        public string Header { get; set; }
        public ICommand Command { get; set; }
        public object CommandParameter { get; set; }
        public IList<MenuItemViewModel> Items { get; set; }
        // Add the Gesture property.
        public KeyGesture Gesture { get; set; }
    }

In MenuPage.xaml:

                <TextBlock Classes="h3" Margin="4 8">Dyanamically generated</TextBlock>
                <Menu Items="{Binding MenuItems}">
                    <Menu.Styles>
                        <Style Selector="MenuItem">
                            <Setter Property="Header" Value="{Binding Header}"/>
                            <Setter Property="Items" Value="{Binding Items}"/>
                            <Setter Property="Command" Value="{Binding Command}"/>
                            <Setter Property="CommandParameter" Value="{Binding CommandParameter}"/>
                            <!-- Add a binding for the HotKey property -->
                            <Setter Property="HotKey" Value="{Binding Gesture}"/>
                        </Style>
                    </Menu.Styles>
                </Menu>

In MenuPageViewModel.cs

                    Items = new[]
                    {
                        new MenuItemViewModel { Header = "_Open...", Command = OpenCommand },
                        // Assign a gesture to the Save command.
                        new MenuItemViewModel
                        {
                            Header = "Save",
                            Command = SaveCommand,
                            Gesture = new Avalonia.Input.KeyGesture(Avalonia.Input.Key.S, Avalonia.Input.InputModifiers.Control)
                        },
                        new MenuItemViewModel { Header = "-" },
                        // The rest of the items....

With these changes, run the ControlCatalog.Desktop project, and go to the Menu page. Press Ctrl+S and notice that nothing will be printed in the output. Open the menu which contains the Save command, and press Ctrl+S again. This time it will print Save in the output.

Gillibald commented 5 years ago

I don't think that these hotkeys should work when the menu isn't open (focused). What you want are global hotkeys(global commands).

Michron commented 5 years ago

If that would be the desired behavior, I would expect that the hotkeys would stop working if the menu is closed again. But they keep working even when the menu is closed again.

Gillibald commented 5 years ago

I think unless you change focus the menu still listens to input gestures. There are plans for a global menu. That should probably always listen to input gestures.

Michron commented 5 years ago

I've ran another test to see what happens if the focus is changed. After opening the menu once it doesn't matter what is focused, the hotkey still works. Even after switching to different pages and while typing inside a TextBox. My guess is that it only stops working if nothing is focused at all, but I didn't find a way to test that in the Control Gallery.

Michron commented 5 years ago

Additionally I've found that if you press the Save command in the menu, and then press Ctrl+S it also doesn't execute the command. It looks like the key up and down event is not consumed by anything despite the MenuItem being focused.

grokys commented 5 years ago

This appears to happen because when using bindings to create the MenuItems, the items are not materialized until the Popup is opened.

I notice that WPF does not have HotKey binding for menu items and you seem to need to do this manually in the containing Window - I think WPF has the same behavior as us here, so this might be the reason for their lack of hotkey handling.

I'll need to have a bit more of a think about potential fixes to this.

grokys commented 5 years ago

@kekekeks is currently refactoring Popups to always dispose of their content when a popup is closed. This is only going to make this worse.

I think the only solution is to remove MenuItem.HotKey and add InputBindings as in WPF.

robloo commented 5 years ago

Instead of InputBindings in might be good to look at the 'updated' concept of KeyboardAccelerator from UWP.

https://docs.microsoft.com/en-us/windows/uwp/design/input/keyboard-accelerators https://docs.microsoft.com/en-us/windows/uwp/design/input/keyboard-interactions

Oh, and then there is gestures for special touch input :)

Raymonf commented 1 year ago

A workaround is to use KeyBindings on the window, if anyone finds this thread in the future:

<Window.KeyBindings>
    <KeyBinding Gesture="Ctrl+O" Command="{Binding OpenCommand}" />
</Window.KeyBindings>
seleborg commented 5 months ago

I use the following method, which also seems to work, and which doesn't require duplicating the keyboard shortcut descriptions:

In MainWindow.xaml:

    <Menu DockPanel.Dock="Top" Name="MainMenu">
      <MenuItem Header="_File">
        <MenuItem Header="_Open..."
                  InputGesture="Ctrl+O"
                  Name="OpenMenuItem"
                  Command="{Binding OpenCommand}"/>
      </MenuItem>
    </Menu>

In MainWindow.xaml.cs:

public partial class MainWindow : Window {
  public MainWindow() {
    InitializeComponent();
  }

  protected override void OnLoaded(RoutedEventArgs _) {
    // At this point, all bindings are resolved, including menu items' .Command properties.
    if (DataContext is MainWindowViewModel vm) {
      RegisterHotKeys(MainMenu);
    }
  }

  void RegisterHotKeys(Control control) {
    if (control is MenuItem item) {
      if (item.InputGesture != null && item.Command != null) {
        KeyBindings.Add(new KeyBinding() {
          Gesture = item.InputGesture,
          Command = item.Command
        });
      }
    }

    foreach (Control child in control.GetLogicalChildren()) {
      RegisterHotKeys(child);
    }
  }
}

This solution assumes that the DataContext doesn't change, otherwise you'd have to unregister and re-register the hotkeys. For a main window, this should work, though.

Tokter commented 1 month ago

Since you closed #16314 as not planned, might as well close this issue. And maybe remove the HotKey property from MenuItem. There is no point in having it if it only works for the most basic of cases.

timunie commented 1 month ago

@Tokter just to clarify, we closed the other issue in favor of this, just because we don't want to have x duplicates of the more or less same issue open .