zadjii-msft / PowerToys

Windows system utilities to maximize productivity
MIT License
3 stars 2 forks source link

Spec allowing keyboard shortcuts for commands #109

Closed joadoumie closed 5 days ago

joadoumie commented 1 month ago

Commands should have a notion of keyboard modifiers as exists in PT Run today:


        public List<ContextMenuResult> LoadContextMenus(Result selectedResult)
        {
            if (!(selectedResult?.ContextData is TerminalProfile))
            {
                return new List<ContextMenuResult>();
            }

            var result = new List<ContextMenuResult>();

            if (selectedResult.ContextData is TerminalProfile profile)
            {
                result.Add(new ContextMenuResult
                {
                    Title = Resources.run_as_administrator,
                    Glyph = "\xE7EF",
                    FontFamily = "Segoe Fluent Icons,Segoe MDL2 Assets",
                    AcceleratorKey = Key.Enter,
                    AcceleratorModifiers = ModifierKeys.Control | ModifierKeys.Shift,
                    Action = _ =>
                    {
                        LaunchElevated(profile.Terminal.AppUserModelId, profile.Name);
                        return true;
                    },
                });
            }

            return result;
        }

This is how existing PT Plugins can input keyboard modifiers / accelerators.

Commands should have specific keyboard modifiers associated with the command.

joadoumie commented 1 month ago

@zadjii-msft I think what would be really cool is if the keyboard modifier logic is baked into Command interface, that way when we can have a bunch of Helper commands that when a developer leverages them in their extensions the keyboard modifiers are the same. This may be exactly what you had in mind already to be fair.

zadjii-msft commented 1 month ago

Eh, I'm not so sure about putting them on Command itself. What happens if a developer puts a shortcut on a Command which is the default action of a TopLevelCommand? Does that make the shortcut work from the L0? (I don't think it should) What if multiple extensions all specify the same shortcut on their L0 commands?

zadjii-msft commented 1 month ago

Though it does look like I deleted this from the "final" version of the spec before I left on leave. I think I mentally bookmarked this as a post-MLP feature.

I'll put this on me to spec out, then a second issue to actually implement it.

zadjii-msft commented 1 month ago

Ah, I did write the following blurb:

https://github.com/zadjii-msft/PowerToys/blob/7b0b298f004247044877805db4341187970545d7/src/modules/cmdpal/doc/initial-sdk-spec/initial-sdk-spec.md?plain=1#L547-L553

joadoumie commented 1 month ago

Totally make sense to have the keyboard binding be to the contextitem imo too.

As much as possible it's really nice if commands that may be reused around all the different surfaces / extensions were to have the same keyboard modifiers... maybe it's as simple as having handy ContextItems that people can reuse through different surfaces that just wrap specific commands that folks also use.