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

core: pass context to when clause for toolbar items #12329

Merged
merged 2 commits into from
Mar 27, 2023
Merged

core: pass context to when clause for toolbar items #12329

merged 2 commits into from
Mar 27, 2023

Conversation

vince-fugnitto
Copy link
Member

What it does

Fixes: #12314
Fixes: #12295 (comment)

The pull-request fixes an issue when evaluating when clauses for toolbar items which were not aware of the current context making the toolbar items never enabled or executable. The change passes the context when it exists to ensure we properly apply the clause.

How to test

  1. confirm the steps from Toolbars: ToolbarItem.when field is evaluated and respected #12067 pass
  2. confirm with vscode-trace-extension-0.1.0.vsix..zip that the trace-viewer view has a toolbar item for "open traces" and that it works well
  3. bonus: confirm that Fix SCM Inline commands #12295 (comment) works

Review checklist

Reminder for reviewers

@vince-fugnitto vince-fugnitto added vscode issues related to VSCode compatibility core issues related to the core of the application toolbar issues related to the toolbar labels Mar 21, 2023
@vince-fugnitto vince-fugnitto requested a review from alvsan09 March 21, 2023 15:49
@vince-fugnitto
core: pass context to when clause
0adf88e 
The commit passes the `context` (which is generally assigned to toolbar
items) in order to determine the when clause for toolbar items.

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
colin-grant-work
colin-grant-work approved these changes Mar 24, 2023
View reviewed changes
Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

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

Confirmed that the problems listed are resolved, and that the code is reasonable on its face.

alvsan09
alvsan09 approved these changes Mar 27, 2023
View reviewed changes
Copy link
Contributor

@alvsan09 alvsan09 left a comment

Choose a reason for hiding this comment

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

The fix works fine !! :+1
There are other issues related to 'editorLangId' and the Toolbar as discusses offline,
Those are being handled as a separate issue.

@vince-fugnitto vince-fugnitto merged commit f3819b2 into master Mar 27, 2023
@vince-fugnitto vince-fugnitto deleted the vf/toolbar-when branch March 27, 2023 15:10
@github-actions github-actions bot added this to the 1.36.0 milestone Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alvsan09 alvsan09 alvsan09 approved these changes

@colin-grant-work colin-grant-work colin-grant-work approved these changes

Assignees
No one assigned
Labels
core issues related to the core of the application toolbar issues related to the toolbar vscode issues related to VSCode compatibility
Projects
None yet
Milestone
1.36.0
Development

Successfully merging this pull request may close these issues.

[vscode] Button in webview title bar not working
3 participants
@vince-fugnitto @colin-grant-work @alvsan09