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

Support ordering of sub menus #7963

Merged
merged 1 commit into from
Jun 5, 2020
Merged

Conversation

Hanksha
Copy link
Contributor

@Hanksha Hanksha commented Jun 4, 2020

What it does

Fixes #7958.
This PR adds the order property to the SubMenuOptions interface used when registering sub menus. The order property is used to sort the sub menus with the menu actions.

Note that this also allows reordering of existing sub menus like this:

menus.registerSubmenu(CommonMenus.File, 'File', {
    order: 'z'
})

How to test

I added a sample file for that called examples/api-samples/src/browser/menu/sample-menu-contribution.ts. This contributes a Sample Menu to the menu bar with order value '2' which should place it right next to the File menu.

image

Review checklist

Reminder for reviewers

@Hanksha Hanksha changed the title [#7958] Support ordering of sub menus Support ordering of sub menus Jun 4, 2020
Signed-off-by: Vivien Jovet <vivien.jovet@gmail.com>
@akosyakov akosyakov added the menus issues related to the menu label Jun 5, 2020
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

code changes look good, tested that sample menu displayed at the proper location

@akosyakov akosyakov merged commit e2f437f into eclipse-theia:master Jun 5, 2020
@Hanksha Hanksha deleted the GH-7958 branch June 5, 2020 08:28
@RomanNikitenko
Copy link
Contributor

@Hanksha @akosyakov
Is it expected to have Sample Menu permanently?
Or it was added for testing goal only?

@Hanksha
Copy link
Contributor Author

Hanksha commented Jun 16, 2020

I just added it to @theia/api-samples for testing and to have an example. Is that a problem?

EDIT: Just read this on the readme

The examples here are all deactivated by default. Commands are provided that toggles on the 
example behavior. These commands are prefixed with "API Sample".

I could try to make the example not visible by default but it will make it more complex. Or put the sub menu somewhere less visible.

@RomanNikitenko
Copy link
Contributor

@Hanksha
it's not a problem
At first sight I just thought that it was added for testing goals only.
thank you for clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
menus issues related to the menu
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add order property to SubMenuOptions
3 participants