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

GH-7103: API for removing toolbar item. #9044

Merged

Conversation

kittaakos
Copy link
Contributor

What it does

This PR adds API for removing toolbar contributions. Closes #7103.

How to test

Review checklist

Reminder for reviewers

Akos Kitta added 2 commits February 9, 2021 13:40
Closes eclipse-theia#7103.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
A `TabBarToolbarContribution` cannot implement `registerToolbarItems` in
an async way.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
Copy link
Contributor

@westbury westbury left a comment

Choose a reason for hiding this comment

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

This looks good to me. In general, for removals, we need to ensure that contributions made by plugins can also be removed, which means keeping a list of removed items that is checked when plugins are installed. However I did not see anything in the plugin API that can add toolbar items so I think we are fine there.

I tested this by removing the ''scm.viewmode.list' item (contributed by the scm package) by calling unregisterItem from the git package.

The second commit to remove the unnecessary async looks good to me too.

@paul-marechal
Copy link
Member

which means keeping a list of removed items that is checked when plugins are installed. However I did not see anything in the plugin API that can add toolbar items so I think we are fine there.

This can be added later if the need comes up.

@paul-marechal paul-marechal merged commit 5236037 into eclipse-theia:master Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

As an extension developer I want unregister tab-bar toolbar contributions
3 participants