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

menu: move away from styler #169748

Merged
merged 2 commits into from
Jan 18, 2023
Merged

menu: move away from styler #169748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2247eb1
menu: move away from styler
aeschli Dec 21, 2022
eceefc9
Merge branch 'main' into aeschli/menuRemoveStyler
aeschli Jan 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 60 additions & 66 deletions src/vs/base/browser/ui/menu/menu.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import { EmptySubmenuAction, IAction, IActionRunner, Separator, SubmenuAction }
import { RunOnceScheduler } from 'vs/base/common/async';
import { Codicon, getCodiconFontCharacters } from 'vs/base/common/codicons';
import { ThemeIcon } from 'vs/base/common/themables';
import { Color } from 'vs/base/common/color';
import { Event } from 'vs/base/common/event';
import { stripIcons } from 'vs/base/common/iconLabels';
import { KeyCode } from 'vs/base/common/keyCodes';
Expand Down Expand Up @@ -50,20 +49,35 @@ export interface IMenuOptions {
}

export interface IMenuStyles {
shadowColor?: Color;
borderColor?: Color;
foregroundColor?: Color;
backgroundColor?: Color;
selectionForegroundColor?: Color;
selectionBackgroundColor?: Color;
selectionBorderColor?: Color;
separatorColor?: Color;
scrollbarShadow?: Color;
scrollbarSliderBackground?: Color;
scrollbarSliderHoverBackground?: Color;
scrollbarSliderActiveBackground?: Color;
shadowColor: string | undefined;
borderColor: string | undefined;
foregroundColor: string | undefined;
backgroundColor: string | undefined;
selectionForegroundColor: string | undefined;
selectionBackgroundColor: string | undefined;
selectionBorderColor: string | undefined;
separatorColor: string | undefined;
scrollbarShadow: string | undefined;
scrollbarSliderBackground: string | undefined;
scrollbarSliderHoverBackground: string | undefined;
scrollbarSliderActiveBackground: string | undefined;
}

export const unthemedMenuStyles: IMenuStyles = {
shadowColor: undefined,
borderColor: undefined,
foregroundColor: undefined,
backgroundColor: undefined,
selectionForegroundColor: undefined,
selectionBackgroundColor: undefined,
selectionBorderColor: undefined,
separatorColor: undefined,
scrollbarShadow: undefined,
scrollbarSliderBackground: undefined,
scrollbarSliderHoverBackground: undefined,
scrollbarSliderActiveBackground: undefined
};

interface ISubMenuData {
parent: Menu;
submenu?: Menu;
Expand All @@ -77,7 +91,7 @@ export class Menu extends ActionBar {
static globalStyleSheet: HTMLStyleElement;
protected styleSheet: HTMLStyleElement | undefined;

constructor(container: HTMLElement, actions: ReadonlyArray<IAction>, options: IMenuOptions = {}) {
constructor(container: HTMLElement, actions: ReadonlyArray<IAction>, options: IMenuOptions, private readonly menuStyles: IMenuStyles) {
container.classList.add('monaco-menu-container');
container.setAttribute('role', 'presentation');
const menuElement = document.createElement('div');
Expand All @@ -101,7 +115,7 @@ export class Menu extends ActionBar {

this.menuDisposables = this._register(new DisposableStore());

this.initializeOrUpdateStyleSheet(container, {});
this.initializeOrUpdateStyleSheet(container, menuStyles);

this._register(Gesture.addTarget(menuElement));

Expand Down Expand Up @@ -229,6 +243,8 @@ export class Menu extends ActionBar {
const scrollElement = this.scrollableElement.getDomNode();
scrollElement.style.position = '';

this.styleScrollElement(scrollElement, menuStyles);

// Support scroll on menu drag
this._register(addDisposableListener(menuElement, TouchEventType.Change, e => {
EventHelper.stop(e, true);
Expand Down Expand Up @@ -278,30 +294,19 @@ export class Menu extends ActionBar {
this.styleSheet.textContent = getMenuWidgetCSS(style, isInShadowDOM(container));
}

style(style: IMenuStyles): void {
const container = this.getContainer();

this.initializeOrUpdateStyleSheet(container, style);
private styleScrollElement(scrollElement: HTMLElement, style: IMenuStyles): void {

const fgColor = style.foregroundColor ? `${style.foregroundColor}` : '';
const bgColor = style.backgroundColor ? `${style.backgroundColor}` : '';
const fgColor = style.foregroundColor ?? '';
const bgColor = style.backgroundColor ?? '';
const border = style.borderColor ? `1px solid ${style.borderColor}` : '';
const borderRadius = '5px';
const shadow = style.shadowColor ? `0 2px 8px ${style.shadowColor}` : '';

container.style.outline = border;
container.style.borderRadius = borderRadius;
container.style.color = fgColor;
container.style.backgroundColor = bgColor;
container.style.boxShadow = shadow;

if (this.viewItems) {
this.viewItems.forEach(item => {
if (item instanceof BaseMenuActionViewItem || item instanceof MenuSeparatorActionViewItem) {
item.style(style);
}
});
}
scrollElement.style.outline = border;
scrollElement.style.borderRadius = borderRadius;
scrollElement.style.color = fgColor;
scrollElement.style.backgroundColor = bgColor;
scrollElement.style.boxShadow = shadow;
}

override getContainer(): HTMLElement {
Expand Down Expand Up @@ -364,9 +369,9 @@ export class Menu extends ActionBar {

private doGetActionViewItem(action: IAction, options: IMenuOptions, parentData: ISubMenuData): BaseActionViewItem {
if (action instanceof Separator) {
return new MenuSeparatorActionViewItem(options.context, action, { icon: true });
return new MenuSeparatorActionViewItem(options.context, action, { icon: true }, this.menuStyles);
} else if (action instanceof SubmenuAction) {
const menuActionViewItem = new SubmenuMenuActionViewItem(action, action.actions, parentData, { ...options, submenuIds: new Set([...(options.submenuIds || []), action.id]) });
const menuActionViewItem = new SubmenuMenuActionViewItem(action, action.actions, parentData, { ...options, submenuIds: new Set([...(options.submenuIds || []), action.id]) }, this.menuStyles);

if (options.enableMnemonics) {
const mnemonic = menuActionViewItem.getMnemonic();
Expand Down Expand Up @@ -396,7 +401,7 @@ export class Menu extends ActionBar {
}
}

const menuActionViewItem = new BaseMenuActionViewItem(options.context, action, menuItemOptions);
const menuActionViewItem = new BaseMenuActionViewItem(options.context, action, menuItemOptions, this.menuStyles);

if (options.enableMnemonics) {
const mnemonic = menuActionViewItem.getMnemonic();
Expand Down Expand Up @@ -433,9 +438,8 @@ class BaseMenuActionViewItem extends BaseActionViewItem {
private check: HTMLElement | undefined;
private mnemonic: string | undefined;
private cssClass: string;
protected menuStyle: IMenuStyles | undefined;

constructor(ctx: unknown, action: IAction, options: IMenuItemOptions = {}) {
constructor(ctx: unknown, action: IAction, options: IMenuItemOptions, protected readonly menuStyle: IMenuStyles) {
options.isMenu = true;
super(action, action, options);

Expand Down Expand Up @@ -542,6 +546,8 @@ class BaseMenuActionViewItem extends BaseActionViewItem {
this.updateTooltip();
this.updateEnabled();
this.updateChecked();

this.applyStyle();
}

override blur(): void {
Expand Down Expand Up @@ -681,32 +687,23 @@ class BaseMenuActionViewItem extends BaseActionViewItem {
}

protected applyStyle(): void {
if (!this.menuStyle) {
return;
}

const isSelected = this.element && this.element.classList.contains('focused');
const fgColor = isSelected && this.menuStyle.selectionForegroundColor ? this.menuStyle.selectionForegroundColor : this.menuStyle.foregroundColor;
const bgColor = isSelected && this.menuStyle.selectionBackgroundColor ? this.menuStyle.selectionBackgroundColor : undefined;
const outline = isSelected && this.menuStyle.selectionBorderColor ? `1px solid ${this.menuStyle.selectionBorderColor}` : '';
const outlineOffset = isSelected && this.menuStyle.selectionBorderColor ? `-1px` : '';

if (this.item) {
this.item.style.color = fgColor ? fgColor.toString() : '';
this.item.style.backgroundColor = bgColor ? bgColor.toString() : '';
this.item.style.color = fgColor ?? '';
this.item.style.backgroundColor = bgColor ?? '';
this.item.style.outline = outline;
this.item.style.outlineOffset = outlineOffset;
}

if (this.check) {
this.check.style.color = fgColor ? fgColor.toString() : '';
this.check.style.color = fgColor ?? '';
}
}

style(style: IMenuStyles): void {
this.menuStyle = style;
this.applyStyle();
}
}

class SubmenuMenuActionViewItem extends BaseMenuActionViewItem {
Expand All @@ -723,9 +720,10 @@ class SubmenuMenuActionViewItem extends BaseMenuActionViewItem {
action: IAction,
private submenuActions: ReadonlyArray<IAction>,
private parentData: ISubMenuData,
private submenuOptions?: IMenuOptions
private submenuOptions: IMenuOptions,
menuStyles: IMenuStyles
) {
super(action, action, submenuOptions);
super(action, action, submenuOptions, menuStyles);

this.expandDirection = submenuOptions && submenuOptions.expandDirection !== undefined ? submenuOptions.expandDirection : Direction.Right;

Expand Down Expand Up @@ -888,10 +886,7 @@ class SubmenuMenuActionViewItem extends BaseMenuActionViewItem {
this.submenuContainer.style.top = '0';
this.submenuContainer.style.left = '0';

this.parentData.submenu = new Menu(this.submenuContainer, this.submenuActions.length ? this.submenuActions : [new EmptySubmenuAction()], this.submenuOptions);
if (this.menuStyle) {
this.parentData.submenu.style(this.menuStyle);
}
this.parentData.submenu = new Menu(this.submenuContainer, this.submenuActions.length ? this.submenuActions : [new EmptySubmenuAction()], this.submenuOptions, this.menuStyle);

// layout submenu
const entryBox = this.element.getBoundingClientRect();
Expand Down Expand Up @@ -951,18 +946,12 @@ class SubmenuMenuActionViewItem extends BaseMenuActionViewItem {
protected override applyStyle(): void {
super.applyStyle();

if (!this.menuStyle) {
return;
}

const isSelected = this.element && this.element.classList.contains('focused');
const fgColor = isSelected && this.menuStyle.selectionForegroundColor ? this.menuStyle.selectionForegroundColor : this.menuStyle.foregroundColor;

if (this.submenuIndicator) {
this.submenuIndicator.style.color = fgColor ? `${fgColor}` : '';
this.submenuIndicator.style.color = fgColor ?? '';
}

this.parentData.submenu?.style(this.menuStyle);
}

override dispose(): void {
Expand All @@ -982,9 +971,14 @@ class SubmenuMenuActionViewItem extends BaseMenuActionViewItem {
}

class MenuSeparatorActionViewItem extends ActionViewItem {
style(style: IMenuStyles): void {
constructor(context: unknown, action: IAction, options: IActionViewItemOptions, private readonly menuStyles: IMenuStyles) {
super(context, action, options);
}

override render(container: HTMLElement): void {
super.render(container);
if (this.label) {
this.label.style.borderBottomColor = style.separatorColor ? `${style.separatorColor}` : '';
this.label.style.borderBottomColor = this.menuStyles.separatorColor ? `${this.menuStyles.separatorColor}` : '';
}
}
}
Expand Down
12 changes: 2 additions & 10 deletions src/vs/base/browser/ui/menu/menubar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,9 @@ export class MenuBar extends Disposable {
private readonly _onFocusStateChange: Emitter<boolean>;

private numMenusShown: number = 0;
private menuStyle: IMenuStyles | undefined;
private overflowLayoutScheduled: IDisposable | undefined = undefined;

constructor(private container: HTMLElement, private options: IMenuBarOptions = {}) {
constructor(private container: HTMLElement, private options: IMenuBarOptions, private menuStyle: IMenuStyles) {
super();

this.container.setAttribute('role', 'menubar');
Expand Down Expand Up @@ -608,10 +607,6 @@ export class MenuBar extends Disposable {
}
}

style(style: IMenuStyles): void {
this.menuStyle = style;
}

update(options?: IMenuBarOptions): void {
if (options) {
this.options = options;
Expand Down Expand Up @@ -1030,10 +1025,7 @@ export class MenuBar extends Disposable {
useEventAsContext: true
};

const menuWidget = this._register(new Menu(menuHolder, customMenu.actions, menuOptions));
if (this.menuStyle) {
menuWidget.style(this.menuStyle);
}
const menuWidget = this._register(new Menu(menuHolder, customMenu.actions, menuOptions, this.menuStyle));

this._register(menuWidget.onDidCancel(() => {
this.focusState = MenubarState.FOCUSED;
Expand Down
3 changes: 2 additions & 1 deletion src/vs/base/test/browser/ui/menu/menubar.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import * as assert from 'assert';
import { $ } from 'vs/base/browser/dom';
import { unthemedMenuStyles } from 'vs/base/browser/ui/menu/menu';
import { MenuBar } from 'vs/base/browser/ui/menu/menubar';

function getButtonElementByAriaLabel(menubarElement: HTMLElement, ariaLabel: string): HTMLElement | null {
Expand Down Expand Up @@ -65,7 +66,7 @@ suite('Menubar', () => {
const menubar = new MenuBar(container, {
enableMnemonics: true,
visibility: 'visible'
});
}, unthemedMenuStyles);

test('English File menu renders mnemonics', function () {
validateMenuBarItem(menubar, container, '&File', 'File', 'F');
Expand Down
4 changes: 1 addition & 3 deletions src/vs/editor/standalone/browser/standaloneServices.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ import { IContextMenuService, IContextViewDelegate, IContextViewService } from '
import { ContextViewService } from 'vs/platform/contextview/browser/contextViewService';
import { LanguageService } from 'vs/editor/common/services/languageService';
import { ContextMenuService } from 'vs/platform/contextview/browser/contextMenuService';
import { IThemeService } from 'vs/platform/theme/common/themeService';
import { getSingletonServiceDescriptors, InstantiationType, registerSingleton } from 'vs/platform/instantiation/common/extensions';
import { OpenerService } from 'vs/editor/browser/services/openerService';
import { IEditorWorkerService } from 'vs/editor/common/services/editorWorker';
Expand Down Expand Up @@ -982,11 +981,10 @@ class StandaloneContextMenuService extends ContextMenuService {
@INotificationService notificationService: INotificationService,
@IContextViewService contextViewService: IContextViewService,
@IKeybindingService keybindingService: IKeybindingService,
@IThemeService themeService: IThemeService,
@IMenuService menuService: IMenuService,
@IContextKeyService contextKeyService: IContextKeyService,
) {
super(telemetryService, notificationService, contextViewService, keybindingService, themeService, menuService, contextKeyService);
super(telemetryService, notificationService, contextViewService, keybindingService, menuService, contextKeyService);
this.configure({ blockMouse: false }); // we do not want that in the standalone editor
}
}
Expand Down
10 changes: 4 additions & 6 deletions src/vs/platform/contextview/browser/contextMenuHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ import { IContextViewService } from 'vs/platform/contextview/browser/contextView
import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
import { INotificationService } from 'vs/platform/notification/common/notification';
import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
import { attachMenuStyler } from 'vs/platform/theme/common/styler';
import { IThemeService } from 'vs/platform/theme/common/themeService';
import { defaultMenuStyles } from 'vs/platform/theme/browser/defaultStyles';


export interface IContextMenuHandlerOptions {
Expand All @@ -34,7 +33,6 @@ export class ContextMenuHandler {
private telemetryService: ITelemetryService,
private notificationService: INotificationService,
private keybindingService: IKeybindingService,
private themeService: IThemeService
) { }

configure(options: IContextMenuHandlerOptions): void {
Expand Down Expand Up @@ -91,9 +89,9 @@ export class ContextMenuHandler {
context: delegate.getActionsContext ? delegate.getActionsContext() : null,
actionRunner,
getKeyBinding: delegate.getKeyBinding ? delegate.getKeyBinding : action => this.keybindingService.lookupKeybinding(action.id)
});

menuDisposables.add(attachMenuStyler(menu, this.themeService));
},
defaultMenuStyles
);

menu.onDidCancel(() => this.contextViewService.hideContextView(true), null, menuDisposables);
menu.onDidBlur(() => this.contextViewService.hideContextView(true), null, menuDisposables);
Expand Down
4 changes: 1 addition & 3 deletions src/vs/platform/contextview/browser/contextMenuService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
import { INotificationService } from 'vs/platform/notification/common/notification';
import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
import { IThemeService } from 'vs/platform/theme/common/themeService';
import { ContextMenuHandler, IContextMenuHandlerOptions } from './contextMenuHandler';
import { IContextMenuMenuDelegate, IContextMenuService, IContextViewService } from './contextView';

Expand All @@ -25,7 +24,7 @@ export class ContextMenuService extends Disposable implements IContextMenuServic
private _contextMenuHandler: ContextMenuHandler | undefined = undefined;
private get contextMenuHandler(): ContextMenuHandler {
if (!this._contextMenuHandler) {
this._contextMenuHandler = new ContextMenuHandler(this.contextViewService, this.telemetryService, this.notificationService, this.keybindingService, this.themeService);
this._contextMenuHandler = new ContextMenuHandler(this.contextViewService, this.telemetryService, this.notificationService, this.keybindingService);
}

return this._contextMenuHandler;
Expand All @@ -42,7 +41,6 @@ export class ContextMenuService extends Disposable implements IContextMenuServic
@INotificationService private readonly notificationService: INotificationService,
@IContextViewService private readonly contextViewService: IContextViewService,
@IKeybindingService private readonly keybindingService: IKeybindingService,
@IThemeService private readonly themeService: IThemeService,
@IMenuService private readonly menuService: IMenuService,
@IContextKeyService private readonly contextKeyService: IContextKeyService,
) {
Expand Down
Loading