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

Avoid changing context key view when changing selection #13768

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

rschnekenbu
Copy link
Contributor

@rschnekenbu rschnekenbu commented Jun 5, 2024

What it does

Closes #13375

In a tree view provided by an extension, refreshing the tree node selection was changing the context key view. This was changing the view in the root context. So when the actions in toolbar were refreshed, the "when" rule was always returning true for any view, not only the concerned one.
Avoid changing the context key view should not be done on selection refresh in this case.

contributed on behalf of STMicroelectronics

How to test

  1. Use the procedure described in 13375
  2. Prior to the patch, when selecting elements in the NPM scripts view, the refresh icon was showing on every view.
  3. With the patch, the refresh action does not show on every view.

Review checklist

Reminder for reviewers

@@ -197,7 +194,6 @@ export class TreeViewsMainImpl implements TreeViewsMain, Disposable {
}));

this.toDispose.push(treeViewWidget.model.onSelectionChanged(event => {
this.contextKeys.view.set(treeViewId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to remove the view context key? It never makes sense to set it in the global context, as I understand it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this context key, as it indeed makes no sense on a global context. focusedView makes sense here, but not view.

fixes eclipse-theia#13375

contributed on behalf of STMicroelectronics

Signed-off-by: Remi Schnekenburger <rschnekenburger@eclipsesource.com>
@rschnekenbu
Copy link
Contributor Author

Pushed a new version that removes view from the view context key, as it makes no sense in a global context.
It solves also usual conflicts on changelog file.

@rschnekenbu rschnekenbu merged commit fadba9c into eclipse-theia:master Jun 27, 2024
14 checks passed
@rschnekenbu rschnekenbu deleted the issues/13375 branch June 27, 2024 08:08
@jfaltermeier jfaltermeier added this to the 1.51.0 milestone Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

NPM Support builtin extension: Issue in the Refresh toolbar command
3 participants