Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VSCode Extension: Disable tree items when enablement clause on linked command is false #102794

Closed
warrenbuckley opened this issue Jul 17, 2020 · 7 comments
Assignees
Labels
feature-request Request for new features or functionality on-testplan tree-views Extension tree view issues
Milestone

Comments

@warrenbuckley
Copy link

Goal/requirement

I am building a VSCode extension and currently creating a new custom view that will have tree items linked to commands.

The goal is to greyout/disable certain tree items when the enablement property on a command evaluates to true false.

This works fine when the commands that are added as menu items to the view and will grey out the icon & will not be able to be clicked, however, I would have expected the same result to happen with the items in the tree.

Screenshot

image

Example code

Package.json

{
    "contributes": {        
        "commands": [
            {
                "command": "extension.iis-express.start",
                "title": "Start Website",
                "category": "IIS Express",
                "enablement": "iisexpress:siterunning != true && isWindows",
                "icon": "$(play)"
            },
            {
                "command": "extension.iis-express.stop",
                "title": "Stop Website",
                "category": "IIS Express",
                "enablement": "iisexpress:siterunning && isWindows",
                "icon": "$(debug-stop)"
            },
            {
                "command": "extension.iis-express.restart",
                "title": "Restart Website",
                "category": "IIS Express",
                "enablement": "iisexpress:siterunning && isWindows",
                "icon": "$(refresh)"
            }
        ],
        "menus": {
            "commandPalette": [
                {
                    "command": "extension.iis-express.start",
                    "when": "iisexpress:siterunning != true && isWindows"
                },
                {
                    "command": "extension.iis-express.stop",
                    "when": "iisexpress:siterunning && isWindows"
                },
                {
                    "command": "extension.iis-express.restart",
                    "when": "iisexpress:siterunning && isWindows"
                }
            ],
            "view/title": [
                {
                    "command": "extension.iis-express.start",
                    "group": "navigation@10"
                },
                {
                    "command": "extension.iis-express.restart",
                    "group": "navigation@11"
                },
                {
                    "command": "extension.iis-express.stop",
                    "group": "navigation@12"
                }
            ]
        },
        "views": {
            "explorer": [
                {
                    "id": "iisexpress.controls",
                    "contextualTitle": "IIS Express",
                    "name": "IIS Express",
                    "when": "isWindows"
                }
            ]
        }
    }
}

My tree menu items are added to the view in the following way

import * as vscode from 'vscode';

export class ControlsTreeProvider implements vscode.TreeDataProvider<ControlsTreeItem> {


    onDidChangeTreeData?: vscode.Event<void | ControlsTreeItem | null | undefined> | undefined;

    getTreeItem(element: ControlsTreeItem): vscode.TreeItem | Thenable<vscode.TreeItem> {
        return element;
    }

    getChildren(element?: ControlsTreeItem | undefined): vscode.ProviderResult<ControlsTreeItem[]> {
        if(element === undefined){
            const items = new Array<ControlsTreeItem>();
            items.push(
                {
                    label: 'Start Website',
                    iconPath: new vscode.ThemeIcon("play"),
                    collapsibleState: vscode.TreeItemCollapsibleState.None,
                    command: {
                        title: 'Start Website',
                        command: "extension.iis-express.start"
                    }
                },
                {
                    label: 'Restart Website',
                    iconPath: new vscode.ThemeIcon("refresh"),
                    collapsibleState: vscode.TreeItemCollapsibleState.None,
                    command: {
                        title: 'Restart Website',
                        command: "extension.iis-express.restart"
                    }
                },
                {
                    label: 'Stop Website',
                    iconPath: new vscode.ThemeIcon("debug-stop"),
                    collapsibleState: vscode.TreeItemCollapsibleState.None,
                    command: {
                        title: 'Stop Website',
                        command: "extension.iis-express.stop"
                    }
                });

            return items;
        }

        return Promise.resolve([]);
    }
}

export class ControlsTreeItem extends vscode.TreeItem {}

And in the main extension entry point

// Register tree provider to put our custom commands into the tree
// Start, Stop, Restart, Support etc...
const controlsTreeProvider = new ControlsTreeProvider();
vscode.window.registerTreeDataProvider('iisexpress.controls', controlsTreeProvider);
@warrenbuckley warrenbuckley changed the title VSCode Extension: Disable tree items when disablement clause on linked command is true VSCode Extension: Disable tree items when enablement clause on linked command is false Jul 17, 2020
@warrenbuckley
Copy link
Author

@sandy081 is this something that the VSCode team would be interested in at all in adding so the experience is more consistent?

@sandy081 sandy081 assigned alexr00 and unassigned sandy081 Jul 23, 2020
@sandy081
Copy link
Member

Forwarding to @alexr00

@alexr00
Copy link
Member

alexr00 commented Jul 23, 2020

I think this makes sense as a feature request. If a tree item has a command and that command's when evaluates to false, then we should style the tree item in a disabled manner.

@alexr00 alexr00 added this to the Backlog milestone Jul 23, 2020
@alexr00 alexr00 added tree-views Extension tree view issues feature-request Request for new features or functionality labels Jul 23, 2020
@warrenbuckley
Copy link
Author

warrenbuckley commented Jul 23, 2020

@alexr00 has this been confirmed as a feature to be put into your own backlog or what happens with the process next?

I may have some time next week to see if I can try & get my hands dirty and do my first PR for this if this is not due to be picked up into your own backlog of work items for a while.

@alexr00
Copy link
Member

alexr00 commented Jul 23, 2020

Full info on how our backlog works in the wiki: https://github.com/microsoft/vscode/wiki/Issues-Triaging#assigning-a-milestone

I like the feature idea but have no plans to do it anytime soon. If you want to make a PR you are welcome to, but this might not be a good first issue. Likely needs changes here https://github.com/Microsoft/vscode/blob/de39b1df90cdadab35fe6f2ee63b7aa0d87e3a90/src/vs/workbench/contrib/views/browser/treeView.ts#L724-L724

@dsanders11
Copy link
Contributor

I think this makes sense as a feature request. If a tree item has a command and that command's when evaluates to false, then we should style the tree item in a disabled manner.

@alexr00, I'm afraid this is a bug, not a feature request, can it be re-labeled and re-prioritized?

enablement does affect view/item/context, the behavior is just buggy. If you explicitly set it to false or use viewItem in the enablement clause you'll see that it does work.

The bug here is it doesn't work when a dynamic context value is used, unless something else causes the node to be re-rendered. It doesn't work if you use the global editorIsOpen context, but view/title does. It doesn't work if you use the setContext command (which is what the example in this issue is using with iisexpress:siterunning), but view/title does.

Here's an easy to reproduce example of why this is a bug. Make a command with editorIsOpen as the enablement, and show the command as an inline in view/item/context, on a tree with children. If you expand a node with children while an editor is open, the icon won't be disabled (correct behavior). If you close the editor, the icons will stay enabled (not correct behavior, violates enablement). Collapse the node and re-expand it, and now the icons are disabled. Anything that causes them to re-render will change the disabled state of the icon. If you scroll up and down the list, any nodes which were previously not visible, or any that go out of view and come back in, will be updated, but others won't, leading to a list with some of the icons disabled and half not.

Here's a GIF showing the buggy behavior:

bug

enablement with dynamic context values also doesn't affect the command palette.

Furthermore, it really seems enablement should not let the command run at all if it's not true. Right now the only thing enforcing enablement appears to be the UI, which means when there are UI bugs like this one, the command can execute even when the developer intended for it to be disabled. The documentation for enablement in the schema says "Condition which must be true to enable the command". With that description it really ought to act like a single source of truth on whether the command can execute or not, otherwise developers are forced to both use enablement and also add those same checks to the command itself to prevent it from being run when UI bugs happen, or if it's invoked with executeCommand.

@alexr00
Copy link
Member

alexr00 commented Aug 10, 2022

Fixed by #157516

@alexr00 alexr00 closed this as completed Aug 10, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Sep 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality on-testplan tree-views Extension tree view issues
Projects
None yet
Development

No branches or pull requests

5 participants