Custom Menubar Implementation #51422
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is just so I can take a screenshot, so the buttons do not work at the moment.
bpasero
reviewed
Jun 15, 2018
| @@ -87,6 +87,10 @@ | |||
| color: inherit; | |||
| } | |||
|
|
|||
| .monaco-menu .monaco-action-bar.vertical .action-label.checked:after { | |||
src/vs/code/electron-main/app.ts
Outdated
| @@ -487,9 +493,6 @@ export class CodeApplication { | |||
| } | |||
| } | |||
|
|
|||
| // Install Menu | |||
| appInstantiationService.createInstance(CodeMenu); | |||
There was a problem hiding this comment.
@sbatten is the functionality from vs/code/electron-main/menus.ts moved over to menubar.ts? then I think it would be safe to remove menu.ts?
However I see that some things are not working yet, like the Accessibility help window, so maybe that is still work in progress?
| @@ -1580,7 +1580,6 @@ export class EditorLayoutCenteredAction extends Action { | |||
| } | |||
|
|
|||
| public run(): TPromise<any> { | |||
|
|
|||
| @@ -24,8 +24,128 @@ | |||
| flex: 0 1 auto; | |||
| overflow: hidden; | |||
| white-space: nowrap; | |||
| line-height: 22px; | |||
bpasero
approved these changes
Jun 17, 2018
There was a problem hiding this comment.
@sbatten I did another code review and pushed some polish to the branch. I think there is one thing to revisit before merging: the changes in ActionBar have some issues:
- we are no longer using
builder.textbutbuilder.innerHTMLfor the label. this increases the risk of someone injecting a HTML script into our DOM because we set it blindly as HTML. we need to continue to not trustIAction.labelas before imho Actionbar#push()seems to do the mnemonic handling now. first of all, we should probably only do that if the action that is pushed hasisMenu: truein the options. Second, there seems to be a map of mnemonics toIActionbut the lifecycle is not clear to me. Actions can come and go in the action bar but this map seems to just grow and never cleanup when an action is removed?
This was referenced Jun 19, 2018
|
@bpasero Hi, there is already a documentation for this api? |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adding custom menubar on windows with themable menus including context menus.