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

VS Code Extensions: Using submenus as "view/title" menu does not work #14072

Closed
Tracked by #13192
chroberino opened this issue Aug 26, 2024 · 4 comments · Fixed by #14132
Closed
Tracked by #13192

VS Code Extensions: Using submenus as "view/title" menu does not work #14072

chroberino opened this issue Aug 26, 2024 · 4 comments · Fixed by #14132
Milestone

Comments

@chroberino
Copy link

Bug Description:

When using a submenu as a "view/title" in a VS Code extension manifest (package.json) the menu items are defunct, i.e. they have no effect when clicking on them. In VS Code the same approach is working fine.

Steps to Reproduce:

  1. Install the Gitlens VS Code extension (eamodio.gitlens) in Theia.
  2. Open the Gitlens Inspect view and navigate to "Search & Compare"
  3. Click on the "plus" icon button (the chevron-down overlay hints there is a submenu, see screenshot).
  4. Click on either action there, it will not have any effect (in VS Code these commands trigger the command prompt)
    grafik

Note

The basic approach in package.json is as follows:

	"submenus": [
		{
			"icon": "$(add)",
			"label": "Add",
			"id": "add_submenu_id"
		}
	],
	"menus": {
		"view/title": [
			{
				"submenu": "add_submenu_id",
				"when": "view == my_custom_view_id",
				"group": "navigation"
			}
		],

		"add_submenu_id": [
			{
				"command": "command_id_0"
			},
			{
				"command": "command_id_1"
			}
		]
	}

(copied from the comments section of this article: https://www.eliostruyf.com/creating-submenu-code-step-step-guide/)

Additional Information

  • Operating System: Windows 10
  • Theia Version: 1.49.1 (self-built browser app)
@msujew
Copy link
Member

msujew commented Aug 26, 2024

This should probably be a part of #13051.

@jonah-iden
Copy link
Contributor

jonah-iden commented Aug 26, 2024

The problem comes from the renderPopupMenumethod in tab-bar-toolbar.ts:360. We are adding the current active widget as an argument for all commands even those contributed by extensions. The current widget can of course not be serialized so it runs into an maximum call stack size exceeded error.

Not sure what commands rely on this current widget being part of the commands args.
Does anyone know if we can just remove this or do we need a more complicated mechanism to distinguish between bakcend and frontend commands?

@tsmaeder
Copy link
Contributor

tsmaeder commented Aug 28, 2024

tab-toolbar.ts#executeCommand has code that adapts the arguments (this.commandExecutor) to send and empty array. However, this code is not invoked when the item in question is in a contributed submenu. T.b.h, I think the whole affair would be simpler if the different argument adapters would be handled where we handle the menu contributions instead of doing it at invocation time.

@tsmaeder tsmaeder mentioned this issue Aug 28, 2024
55 tasks
@tsmaeder
Copy link
Contributor

tsmaeder commented Aug 28, 2024

This disable any action in contributed toolbar drop-down. I believe we should address this issue in a timely fashion. A fix should not take more than 2 days.

tsmaeder added a commit to tsmaeder/theia that referenced this issue Sep 5, 2024
Fixes eclipse-theia#14072

Contributed on behalf of STMicroelectronics

Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>om>
tsmaeder added a commit that referenced this issue Sep 5, 2024
Fixes #14072

Contributed on behalf of STMicroelectronics

Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>om>
jfaltermeier pushed a commit that referenced this issue Sep 6, 2024
Fixes #14072

Contributed on behalf of STMicroelectronics

Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>om>
@sgraband sgraband added this to the 1.54.0 milestone Sep 26, 2024
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 a pull request may close this issue.

7 participants
@msujew @tsmaeder @jonah-iden @sgraband @chroberino and others