core: added appearance and editor layout sub-menus #10220

Archie27376 · 2021-10-04T16:13:01Z

What it does

The commit adds the registration of an appearance and editor layout submenu to the view main-menu.
The appearance menu is used to group menu items related to the appearance of the workbench or layout.
The editor layout menu groups items related to splitting the workbench.
Commands present in vscode's appearance and editor layout menu that exist in the framework were moved to these new submenus.

How to test

Follow steps to open the Electron app:

yarn electron build
yarn
yarn electron start

Click on the View menu from the top bar and then the Appearance and Editor Layout sub-menus within. The list of options will be present, click on each one to test their features, but some require prerequisites to be useful.

Review checklist

As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

As a reviewer, I agree to behave in accordance with the review guidelines

colin-grant-work · 2021-10-05T15:22:02Z

These changes are a good start. I wonder if you're interested in adding additional commands to the new 'Appearance' menu to bring it closer to VSCode? Here's what they've got and what you've added so far:

Some of the things on the VSCode menu don't make any sense for us: moving the side bar right or the panel left don't really mean anything for us. I think we can add a command for 'Show Menu Bar' now that we support the window.menuBarVisibility preference, though, and we have CommonCommands.TOGGLE_MAXIMIZED for something close to Zen Mode.

packages/core/src/electron-browser/menu/electron-menu-contribution.ts

packages/core/src/browser/common-frontend-contribution.ts

Archie27376 · 2021-10-22T20:14:01Z

@msujew I was wondering if you are able to help me out with localizing this PR and I am not sure how to acquire the keys.

msujew

@Archie27376 Thanks for asking. I usually just look up the english text in the vscode codebase (which I simply opened using github.dev or pressing . while in the vscode github repo) and build the translation key based on that: vscode/<filename>/<id>, where the id is the ID used in the vscode localize call, and the filename is the file (without extension) that contains that call.

I'm currently building a tool to simplify that process, but for now that's the simplest way that I found. Anyway, I helped you with that (since it's kind of my fault, that the workflow for translating Theia - at least for now - is so complicated), see the suggestions.

packages/editor/src/browser/editor-menu.ts

packages/core/src/electron-browser/menu/electron-menu-contribution.ts

packages/core/src/browser/common-frontend-contribution.ts

packages/editor/src/browser/editor-menu.ts

Archie27376 · 2021-10-28T15:22:08Z

Would someone be able to review this PR?

colin-grant-work · 2021-11-04T15:47:23Z

@Archie27376, there are a couple of ways to get people's attention. Using an @ makes it more likely that it'll show up in someone's notifications, and requesting a review from someone using using the reviewers widget in the upper-right area will make sure that it shows up in the reviewer's 'PR's awaiting review' queue.

colin-grant-work · 2021-11-05T15:25:30Z

@Archie27376, please rebase this, and you may be able to make use of some of the new localization utilities that have been added by #10319 .

msujew

There are some nitpicks regarding the localizations, everything else looks good.

packages/core/src/browser/common-frontend-contribution.ts

packages/editor/src/browser/editor-menu.ts

packages/core/src/browser/common-frontend-contribution.ts

colin-grant-work

One funny behavior that I see in Electron with Native menus is that the Editor Layout submenu may not show up if you start the application with no editors. It will show up if you open an editor and then change menu modes (now easily available from the appearance menu :-)). Since that behavior might be strange to users who don't understand the perils of Electron's native menus, it might be best to make the split editor commands always enabled and execution just a no-op if there's nothing to split.

colin-grant-work

The code looks good to me, and the behavior of the menu in Electron with native menus is correct.

msujew

The changes look good to me as well. @colin-grant-work merge at your discretion :)

colin-grant-work · 2021-12-07T17:08:18Z

@Archie27376, sorry I've left this long enough that you'll need to rebase. Ping me when you're done, and I'll merge.

msujew · 2021-12-15T19:00:36Z

@colin-grant-work ping :)

colin-grant-work · 2021-12-15T19:05:57Z

packages/editor/src/browser/editor-contribution.ts

+            isEnabled: () => !!this.editorManager.currentEditor,
+            execute: async () => {
+                const { currentEditor } = this.editorManager;
+                if (currentEditor) {
+                    const selection = currentEditor.editor.selection;
+                    const newEditor = await this.editorManager.openToSide(currentEditor.editor.uri, { selection, widgetOptions: { mode: splitMode } });
+                    const oldEditorState = currentEditor.editor.storeViewState();


It looks like you've resolved the conflict here in favor of your code, but the changes made in the conflicting commit were significant - without them, the editor tab context menu will be broken. I think your change was made to ensure that this item would be visible in electron even if no editor was present at startup. To achieve that, please keep the old code, but remove the isVisible and isEnabled checks.

Archie27376 · 2021-12-17T20:23:09Z

@colin-grant-work ping

packages/core/src/browser/common-frontend-contribution.ts

The commit adds the registration of an `appearance` and `editor layout` sub-menu to the `view` main menu. Names have also been localized. The `appearance` menu is used to group menu items related to the appearance of the workbench or layout. The `editor layout` menu groups items related to splitting the workbench. Commands present in vscode's appearance and editor layout menu that exist in the framework were moved to the new sub-menus.

colin-grant-work

The View menu is much improved, and the other menus continue to show what they should. 👍

Archie27376 changed the title ~~Appearance: option added in view tab~~ core: added appearance sub-menu Oct 4, 2021

Archie27376 force-pushed the Appearance_Option branch from fbe5375 to 449a07a Compare October 4, 2021 16:31

vince-fugnitto added the menus issues related to the menu label Oct 4, 2021

Archie27376 force-pushed the Appearance_Option branch from 449a07a to 4840466 Compare October 18, 2021 19:49

vince-fugnitto reviewed Oct 18, 2021

View reviewed changes

packages/core/src/electron-browser/menu/electron-menu-contribution.ts Outdated Show resolved Hide resolved

packages/core/src/browser/common-frontend-contribution.ts Show resolved Hide resolved

packages/core/src/browser/common-frontend-contribution.ts Outdated Show resolved Hide resolved

Archie27376 force-pushed the Appearance_Option branch 3 times, most recently from ed15ed6 to 67d8cde Compare October 22, 2021 16:32

Archie27376 changed the title ~~core: added appearance sub-menu~~ core: added appearance and editor layout sub-menus Oct 22, 2021

Archie27376 force-pushed the Appearance_Option branch 6 times, most recently from 9ec6935 to e6a17a0 Compare October 22, 2021 19:03

msujew reviewed Oct 22, 2021

View reviewed changes

Archie27376 force-pushed the Appearance_Option branch from e6a17a0 to 03eaa69 Compare October 25, 2021 14:23

colin-grant-work self-requested a review November 4, 2021 15:47

Archie27376 force-pushed the Appearance_Option branch 2 times, most recently from 1f4c3fc to 2850034 Compare November 15, 2021 16:50

msujew reviewed Nov 15, 2021

View reviewed changes

Archie27376 force-pushed the Appearance_Option branch from 2850034 to 4e00abd Compare November 15, 2021 17:53

colin-grant-work reviewed Nov 18, 2021

View reviewed changes

Archie27376 force-pushed the Appearance_Option branch from 4e00abd to d3c445f Compare November 22, 2021 16:58

Archie27376 requested a review from colin-grant-work November 24, 2021 19:35

colin-grant-work approved these changes Nov 24, 2021

View reviewed changes

Archie27376 requested review from msujew and vince-fugnitto November 30, 2021 20:41

msujew approved these changes Dec 7, 2021

View reviewed changes

Archie27376 force-pushed the Appearance_Option branch 2 times, most recently from 0bd86b2 to bcc1013 Compare December 15, 2021 17:55

colin-grant-work reviewed Dec 15, 2021

View reviewed changes

Archie27376 force-pushed the Appearance_Option branch from bcc1013 to 3fc564a Compare December 17, 2021 20:21

colin-grant-work reviewed Dec 17, 2021

View reviewed changes

packages/core/src/browser/common-frontend-contribution.ts Show resolved Hide resolved

Archie27376 force-pushed the Appearance_Option branch from 3fc564a to a413dc8 Compare December 17, 2021 21:20

colin-grant-work approved these changes Dec 17, 2021

View reviewed changes

colin-grant-work merged commit b1778c3 into eclipse-theia:master Dec 17, 2021

vince-fugnitto added this to the 1.22.0 milestone Jan 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core: added appearance and editor layout sub-menus #10220

core: added appearance and editor layout sub-menus #10220

Archie27376 commented Oct 4, 2021 •

edited

Loading

colin-grant-work commented Oct 5, 2021 •

edited

Loading

Archie27376 commented Oct 22, 2021

msujew left a comment •

edited

Loading

Archie27376 commented Oct 28, 2021

colin-grant-work commented Nov 4, 2021

colin-grant-work commented Nov 5, 2021

msujew left a comment

colin-grant-work left a comment

colin-grant-work left a comment

msujew left a comment

colin-grant-work commented Dec 7, 2021

msujew commented Dec 15, 2021

colin-grant-work Dec 15, 2021 •

edited

Loading

Archie27376 commented Dec 17, 2021

colin-grant-work left a comment

core: added appearance and editor layout sub-menus #10220

core: added appearance and editor layout sub-menus #10220

Conversation

Archie27376 commented Oct 4, 2021 • edited Loading

What it does

How to test

Review checklist

Reminder for reviewers

colin-grant-work commented Oct 5, 2021 • edited Loading

Archie27376 commented Oct 22, 2021

msujew left a comment • edited Loading

Choose a reason for hiding this comment

Archie27376 commented Oct 28, 2021

colin-grant-work commented Nov 4, 2021

colin-grant-work commented Nov 5, 2021

msujew left a comment

Choose a reason for hiding this comment

colin-grant-work left a comment

Choose a reason for hiding this comment

colin-grant-work left a comment

Choose a reason for hiding this comment

msujew left a comment

Choose a reason for hiding this comment

colin-grant-work commented Dec 7, 2021

msujew commented Dec 15, 2021

colin-grant-work Dec 15, 2021 • edited Loading

Choose a reason for hiding this comment

Archie27376 commented Dec 17, 2021

colin-grant-work left a comment

Choose a reason for hiding this comment

Archie27376 commented Oct 4, 2021 •

edited

Loading

colin-grant-work commented Oct 5, 2021 •

edited

Loading

msujew left a comment •

edited

Loading

colin-grant-work Dec 15, 2021 •

edited

Loading