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.

Sign up for GitHub

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

Jump to bottom

NPM Support builtin extension: Issue in the Refresh toolbar command #13375

Closed
Tracked by #13192
safisa opened this issue Feb 12, 2024 · 6 comments · Fixed by #13768
Closed
Tracked by #13192

NPM Support builtin extension: Issue in the Refresh toolbar command #13375

safisa opened this issue Feb 12, 2024 · 6 comments · Fixed by #13768
Assignees
@rschnekenbu
Labels
bug bugs found in the application help wanted issues meant to be picked up, require help vscode issues related to VSCode compatibility
Milestone
1.51.0

Comments

@safisa
Copy link
Contributor

safisa commented Feb 12, 2024

Bug Description:

Hi,

There is an issue in the builtin vscode NPM support extension, where the refresh command could be added to other tree components like the file explorer and the extensions list.

Steps to Reproduce:

  1. have a project with some package.json files
  2. open the npm view
  3. expand a package.json item to see its scripts
  4. collapse it back
  5. you will see that a new refresh icon is added to the navigator tree, also to the whole explorer view, and to the extensions view...
  6. if you do inspect on this icon, you will see that it comes from the npm.refresh command

Additional Information

  • Operating System: Mac
  • Theia Version: 1.45.1
@safisa
Copy link
Contributor Author

safisa commented Feb 12, 2024 •
edited
Loading

This could be related to the view/title menu contribution with this condition: "when": "view == npm"

@JonasHelming JonasHelming mentioned this issue Feb 13, 2024
Open
45 tasks
@msujew msujew mentioned this issue Mar 15, 2024
Closed
@msujew msujew added bug bugs found in the application help wanted issues meant to be picked up, require help vscode issues related to VSCode compatibility labels Mar 15, 2024
@vnfedotov
Copy link
Contributor

Are there any workarounds for this issue? I've tried multiple ways of rewriting "when" predicate, but it doesn't seem to work.

@msujew
Copy link
Member

msujew commented Mar 20, 2024

@vnfedotov I assume this is a bug in the update logic of the toolbar renderer. I'm not aware of a workaround and I don't think there is one. Feel free to contribute a PR to fix the issue.

@msujew msujew mentioned this issue Apr 11, 2024
Closed
rschnekenbu added a commit to eclipsesource/theia that referenced this issue Jun 5, 2024
@rschnekenbu
Avoid changing context key view when changing selection
f862f23 
fixes eclipse-theia#13375

contributed on behalf of STMicroelectronics

Signed-off-by: Remi Schnekenburger <[email protected]>
@rschnekenbu rschnekenbu mentioned this issue Jun 5, 2024
Merged
1 task
rschnekenbu added a commit to eclipsesource/theia that referenced this issue Jun 5, 2024
@rschnekenbu
Avoid changing context key view when changing selection
e43ebd5 
fixes eclipse-theia#13375

contributed on behalf of STMicroelectronics

Signed-off-by: Remi Schnekenburger <[email protected]>
@rschnekenbu rschnekenbu self-assigned this Jun 5, 2024
@tsmaeder
Copy link
Contributor

@rschnekenbu to quote @msujew

@vnfedotov I assume this is a bug in the update logic of the toolbar renderer.

and I tend to agree: why would setting the context key lead to the icons being added to the wrong view?

@msujew
Copy link
Member

msujew commented Jun 19, 2024

@tsmaeder I believe the erroneous logic is explained pretty well in #13768. At least it sounds pretty reasonable to me.

@tsmaeder
Copy link
Contributor

Hmh...what I mean is this: the context key view does not make sense as a global value: "which view is this" is always local. Contrary to activeViewlet, etc. The local, scoped context is set, for example in plugin-view-widget.ts#77, but it's not using the constant from view-context-service.ts. If it's not supposed to be used, shouldn't we at least remove the constant?

rschnekenbu added a commit to eclipsesource/theia that referenced this issue Jun 26, 2024
@rschnekenbu
Avoid changing context key view when changing selection
95ecea5 
fixes eclipse-theia#13375

contributed on behalf of STMicroelectronics

Signed-off-by: Remi Schnekenburger <[email protected]>

address review suggestion: remove view from context key service
rschnekenbu added a commit to eclipsesource/theia that referenced this issue Jun 26, 2024
@rschnekenbu
Avoid changing context key view when changing selection
29aea5b 
fixes eclipse-theia#13375

contributed on behalf of STMicroelectronics

Signed-off-by: Remi Schnekenburger <[email protected]>
rschnekenbu added a commit that referenced this issue Jun 27, 2024
@rschnekenbu
Avoid changing context key view when changing selection (#13768)
fadba9c 
fixes #13375

contributed on behalf of STMicroelectronics

Signed-off-by: Remi Schnekenburger <[email protected]>
@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
Assignees

@rschnekenbu rschnekenbu

Labels
bug bugs found in the application help wanted issues meant to be picked up, require help vscode issues related to VSCode compatibility
Projects
None yet
Milestone
1.51.0
Development

Successfully merging a pull request may close this issue.

Avoid changing context key view when changing selection eclipsesource/theia
6 participants
@vnfedotov @rschnekenbu @msujew @jfaltermeier @safisa @tsmaeder