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

Improve plugin-view toolbar lifecycle #10546

Merged
merged 2 commits into from
Jan 6, 2022

Conversation

colin-grant-work
Copy link
Contributor

@colin-grant-work colin-grant-work commented Dec 15, 2021

What it does

Fixes #9075.
Closes #9080.
This PR builds on #9080. That PR addressed a bug in the fetching of values from workspace scope in the MonacoConfigurationService, but did not fix the problem with missing events for updating toolbars in plugin-contributed views. This PR makes several changes to address the latter problem:

  • Removes the .with utility from the ViewContextKeyService. That utility was designed to check whether a when expression's conditions would be satisfied given certain assumptions, but it caused two or four change events to be fired by the ContextKeyService for every check. It has been replaced by a .with utility on the MonacoContextKeyService class that uses Monaco utilities to create subcontexts so that the events don't bubble to the global context.
  • Adds a local context to plugin view widgets so that tabbar toolbar checks no longer need to use the .with utility at all.
  • Separates the ContextKeyService interface from the mostly useless dummy implementation in core. This is a breaking change (extenders can no longer extends ContextKeyService), but it allows us to document the functionality of the methods. Previously each method was just annotated with Should be implemented elsewhere.

How to test

  • Add the Maven plugin.
  • Click on the flat / hierarchical item.
  • Observe that when you click, it changes to the other item consistently.
  • Check that other tabbar toolbar items and context-dependent items are working as expected.

Review checklist

Reminder for reviewers

@colin-grant-work colin-grant-work added monaco issues related to monaco plug-in system issues related to the plug-in system labels Dec 15, 2021
@colin-grant-work colin-grant-work force-pushed the bugfix/tree-view-toolbars branch from 810089e to 77282e5 Compare December 15, 2021 21:30
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm that the issue exists on master and is addressed by this change.

  • The file tree/list icon correctly changes using the maven extension
  • Other buttons remain working as expected (no regressions)

The code looks good to me 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
monaco issues related to monaco plug-in system issues related to the plug-in system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Icons (and Their Matching Commands ) of view/title Do not Update Upon Changes in Workspace settings.json
3 participants