-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Track visible viewlet #12597
Track visible viewlet #12597
Conversation
Track active top level widget and set the relevant context keys if the visible widget is either well known or a contributed view container. Contributed on behalf of STMicroelectronics Signed-off-by: Thomas Mäder <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've noticed that terminal widgets aren't tracked correctly by this change (they also weren't tracked by the old implementation). You will likely need to handle the terminal case in a special manner, since terminal widgets don't use the TERMINAL_WIDGET_FACTORY_ID
as their id
. For example, the first terminal the application spawns has id: 'terminal-0'
, with every subsequent terminal just incrementing that number.
Everything else looks fine to me though 👍
That's on purpose: VS Code has a single terminal view with multiple terminals running inside it. Theia has multiple terminal views. As a consequence "the terminal view is active" does not really make sense in Theia. |
Ok, I can agree with that. However, the code itself doesn't really say that, and is more in line with my expectation. Maybe add a comment in there explaining that the current terminal widget will never hit that mapping and should be revised when a consolidated terminal widget like in vscode has been introduced to the framework? |
Signed-off-by: Thomas Mäder <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, looks good to me 👍
What it does
Track active top level widget and set the relevant context keys if the visible widget is either well known or a contributed view container.
Fixes #12589
Contributed on behalf of STMicroelectronics
How to test
plugin-frontend-contribution.ts
:Review checklist
Reminder for reviewers