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

Explorer and open editors should use enablement via menu service #41103

Closed
isidorn opened this issue Jan 3, 2018 · 3 comments
Closed

Explorer and open editors should use enablement via menu service #41103

isidorn opened this issue Jan 3, 2018 · 3 comments
Assignees
Labels
feature-request Request for new features or functionality menus Menu items and widget issues verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@isidorn
Copy link
Contributor

isidorn commented Jan 3, 2018

In the current stable vscode we use enablement state to show if an action is applicable on the current context in the open editors and explorer menu.
In the current insiders we simply do not show the actions which are not applicabe since we have adopted the open editors and explorer to use the menu service. And we use the when clause.

We should look into if it is possible for the MenuService to expose an enablement clause.

@jrieken is this already exposed? I could not find it
fyi @bpasero

@isidorn isidorn added this to the December 2017/January 2018 milestone Jan 3, 2018
@isidorn isidorn self-assigned this Jan 3, 2018
@jrieken
Copy link
Member

jrieken commented Jan 4, 2018

The idea is that a command has a precondition which is must be fulfilled in order to run. Editor commands already have that. An example is that the format action cannot run a readonly-document.

For commands, I did add ICommand#precondition but the enforcement must happen in the right context (not how it's currently done). In essence, the user-gesture must decide how to handle the precondition, e.g. a menu item shows it as disabled, and it must do that in the same context its uses for visibility.

@jrieken jrieken self-assigned this Jan 4, 2018
@jrieken jrieken added the menus Menu items and widget issues label Jan 5, 2018
jrieken added a commit that referenced this issue Jan 9, 2018
jrieken added a commit that referenced this issue Jan 9, 2018
@jrieken
Copy link
Member

jrieken commented Jan 9, 2018

@isidorn scrap this comment... I undone ICommand#precondition and I have added ICommandAction#precondition and adopted it in MenuItemAction. Now, when registering a command you can define its precondition and when calling Menu#getActions the returned actions will have the state set properly.

@jrieken jrieken added the feature-request Request for new features or functionality label Jan 9, 2018
@isidorn isidorn closed this as completed in e5cbf75 Jan 9, 2018
@isidorn
Copy link
Contributor Author

isidorn commented Jan 9, 2018

@jrieken thanks for this, it works great
@bpasero I compared with the current stable and added some preconditions (for save, compare with saved, revert paste). I had to introduce a dirtyEditor context for this.
Let me know if you have more precondition ideas.

@isidorn isidorn added the verification-needed Verification of issue is requested label Jan 9, 2018
@roblourens roblourens added the verified Verification succeeded label Feb 1, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Feb 23, 2018
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 menus Menu items and widget issues verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants