zadjii-msft / PowerToys

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

Make sure extensions can change the context menu, from the context menu #153

Closed zadjii-msft closed 1 day ago

zadjii-msft commented 1 week ago

Example:

  1. You've got the GitHub extension, with a list of issues.
  2. One of the context entries is "Assign issue".
  3. When the user invokes that action, the menu changes to list users in the repo
  4. The user can then select one of those people

Notes from teams - this was something we discussed on a Friday then never wrote down

Okay so this I can finally think about and I suspect it's not impossible with the current API design. There's no reason a command in the MoreCommands list couldn't call back to the ListItem, and change it's MoreCommands,

Now, that's also got some goofiness to it:

  • we'd need to have the flyout stay open or be able to be re-opened. So that the changes to the menu are immediately apparent
  • we'd need to have a way to have the extension know when the menu was dismissed (so that it could revert back to the original state)
  • and how does this play with "we show the DefaultCommand and the first N MoreCommand's as buttons next to the ..."

Alternatively, maybe we could add another API on top of that which like specifically opens a new MoreCommands context menu. We'd have to definitely spec out how a command requests this, and what if like, a ListPage calls that APi when there's not a context menu opened? we'd really have to limit the scope of that API to "there is an active context menu". Maybe that could be an API (Host.GetCurrentContextMenu() or something)


other notes

In 3, it could return StayOpen, and then the parent ListItem raises a PropChanged(MoreCommands). Of course, the real problem here is that there's no existing way to tell the IListCommand that the menu dismissed, so the app would need to build in another way to go back to the old menu.

zadjii-msft commented 4 days ago

OR

We could have a


[uuid("c78b9851-e76b-43ee-8f76-da5ba14e69a4")]
interface IContextItem {}

interface ICommandContextItem requires IContextItem {
    ICommand Command { get; };
    String Tooltip { get; };
    Boolean IsCritical { get; };
}

interface ISubMenuContextItem requires IContextItem {
    IContextItem[] MoreCommands{ get; };
    // These two are from ICommand....
    String Name;
    IconDataType Icon;
    // These ones are lightly reused from ICommandContextItem - should dedupe
    String Tooltip { get; };
    Boolean IsCritical { get; };
}

So then, if the user invokes a ISubMenuContextItem, we just build a new menu there. And dismissing the menu in that state allows us to go back to the root context menu

zadjii-msft commented 3 days ago

Is ListItem.ContextMenu also just a list of ILIstItems?

and if you hit enter on one, then we'd


I think I finally understand. (alternatively, I haven't had enough sleep and this is ravings of a madman)

what weird stuff on IListItem wouldn't apply to a page?

zadjii-msft commented 3 days ago

Do we just:

Cause THEN the TopLevelCommands that we'd stash as stubs is literally just an ICommand - a {Id, ShortName, Title, Subtitle, Icon}. Everything else either:

i'm a genius

zadjii-msft commented 3 days ago

When displaying a list item:

A stub list item is:

StubItem(){ 
    Id = IListItem.Command.Id,
    Icon = IListItem.Icon(), 
    Title = IListItem.Title(), 
    Subtitle = IListItem.Subtitle, 
    ShortName = IListItem.Command.ShortName,
}

(resolving icon and title as detailed above)

When displaying a command context menu item (not nested):

When displaying the default command context item, we'll make a

ICommandContextItem(){ 
    Command = IListItem.Command,
    Icon = IListItem.Command.Icon, 
    Title = IListItem.Command.ShortName, 
    Tooltip = IListItem.Title, 
    IsCritical = false,
}

When displaying a nested context menu item:

When displaying a page:

And now that I've typed this all out - does it make sense for ICommand to have an icon? If IListItem, and ICommandContextItem both have icons, why not just give IPage one too, and remove all the icon fallback machinery?

zadjii-msft commented 3 days ago

Plan V5:

interface ICommand requires INotifyPropChanged{
    String ShortName{ get; };
    String Id{ get; };
    IconDataType Icon{ get; };
}
interface IPage requires ICommand {
    String Title { get; };
    Boolean Loading { get; };
}

interface ICommandItem requires INotifyPropChanged {
    ICommand Command{ get; };
    IContextItem[] MoreCommands{ get; };
    IconDataType Icon{ get; };
    String Title{ get; };
    String Subtitle{ get; };
}    

interface IListItem requires ICommandItem {
    ITag[] Tags{ get; };
    IDetails Details{ get; };
    IFallbackHandler FallbackHandler{ get; };
    String Section { get; };
    String TextToSuggest { get; };
}

interface ICommandContextItem requires ICommandItem, IContextItem {
    Boolean IsCritical { get; }; // READ: "make this red"
}

interface IFallbackCommandItem requires ICommandItem {
    IFallbackHandler FallbackHandler{ get; };
} 

interface ICommandProvider requires Windows.Foundation.IClosable
{
    // ...
    ICommandItem[] TopLevelCommands();
    IFallbackCommandItem[] FallbackCommands();
    ICommand GetCommand(String id);
    void InitializeWithHost(IExtensionHost host);
};

What if IListItem and ICommandItem shared the same base type? Basically, they're both a list of things which have

If we structure it like this, then the TopLevelCommands are just ICommandItems. That's the thing we can serialize to a stub. AND by putting MoreCommands on ICommandItem, top level commands can have context menus (which are just more ICommandItem)

All the bits of a list item which we can't have on a stub? They still live in IListItem, which we don't use in the toplevel.

Fallback handlers have to move. They already felt weird on IListItem itself. Instead, we'll stick them as a separate property on ICommandProvider. This lets us keep fallbacks entirely separate from normal top-level items. It gives us a better abstraction for enabling/disabling fallbacks from a provider. It lets us clearly identify providers that need to be treated as fresh, never frozen, because of the presence of fallbacks. And if an extension's own list page wants to implement a similar fallback mechanism - it's free to use IDynamicListPage to listen for changes to the query and have it's own ListItem it updates manually.

When displaying a list item:

When displaying a command context menu item:

When displaying a IListItem's default Command as a context item, we'll make a new

ICommandContextItem(){ 
    Command = ICommandItem.Command,
    MoreCommands = null,
    Icon = Command.Icon, // use icon from command, not list item 
    Title = Command.Name, // Use command's name, not list item
    Subtitle = IListItem.Title, // Use the title of the list item as the tooltip on the context menu
    IsCritical = false,
}

If a ICommandItem in a context menu has MoreCommands, then activating it will open a submenu with those items. If a ICommandItem in a context menu has MoreCommands AND a non-null Command, then activating it will open a submenu with the Command first (following the same rules above for building a context item from a default Command), followed by the items in MoreCommands.

When displaying a page:

zadjii-msft commented 3 days ago

Summary

TL;DR: I'm gonna touch all the extensions, and make IListItem and ICommandContextItem share a root interface. TopLevelCommands() won't return IListItems, but instead ICommandItems, and it's all gonna be simpler.

Alright so I'm planning one last largish, breaking-ish change to the API.

Right now we've got IListItems, which have a ICommand, as well as a title/subtitle/icon. And those list items might have a context menu of MoreCommands, which are IContextItems (which mostly just also have a ICommand, title, icon, tooltip etc.)

We've also got a wacky situation where we want to store "stubs" for top-level commands, so that we don't always need to instantiate the extension process just to read top level commands from it. So some bits of IListItem (like details, sections, tags, TextToSuggest, FallbackHandler)... they don't really make sense on TopLevelCommand ListItems. Only really the title,subtitle,icon,name matter.

Well ListViews and ContextMenus, those are also basically just the same thing - a list of commands to display. So I'm gonna consolidate classes slightly. Here's a side-by-side diff

Before After
```diff -interface IListItem requires INotifyPropChanged { - IconDataType Icon{ get; }; - String Title{ get; }; - String Subtitle{ get; }; - ICommand Command{ get; }; - IContextItem[] MoreCommands{ get; }; ITag[] Tags{ get; }; IDetails Details{ get; }; - IFallbackHandler FallbackHandler{ get; }; String Section { get; }; String TextToSuggest { get; }; } interface IContextItem {} interface ISeparatorContextItem requires IContextItem {} -interface ICommandContextItem requires IContextItem { - ICommand Command { get; }; - IconDataType Icon{ get; }; - String Title{ get; }; - String Tooltip { get; }; Boolean IsCritical { get; }; // READ: "make this red" } -interface ISubMenuContextItem requires IContextItem { - IContextItem[] MoreCommands{ get; }; - IconDataType Icon{ get; }; - String Title{ get; }; - String Tooltip { get; }; - Boolean IsCritical { get; }; // READ: "make this red" -} interface ICommandProvider requires Windows.Foundation.IClosable { // ... - IListItem[] TopLevelCommands(); ICommand GetCommand(String id); void InitializeWithHost(IExtensionHost host); }; ``` ```diff + interface ICommandItem requires INotifyPropChanged { + ICommand Command{ get; }; + IContextItem[] MoreCommands{ get; }; + IconDataType Icon{ get; }; + String Title{ get; }; + String Subtitle{ get; }; + } +interface IListItem requires ICommandItem { ITag[] Tags{ get; }; IDetails Details{ get; }; String Section { get; }; String TextToSuggest { get; }; } +interface ICommandContextItem requires ICommandItem, IContextItem { Boolean IsCritical { get; }; // READ: "make this red" } +interface IFallbackCommandItem requires ICommandItem { + IFallbackHandler FallbackHandler{ get; }; +} interface ICommandProvider requires Windows.Foundation.IClosable { // ... + ICommandItem[] TopLevelCommands(); + IFallbackCommandItem[] FallbackCommands(); ICommand GetCommand(String id); void InitializeWithHost(IExtensionHost host); }; ```

This makes the API way less repetitive. More of the currency of the app is now just ICommandItems throughout. It aligns what can be present on stub top-level items with what's actually there in the API.