From 85bac873e77c186ceaa457e061def1ece68b3212 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Thu, 22 Aug 2024 16:03:33 +0200 Subject: [PATCH] fix: touch bar functionality on BaseWindow (#43422) * fix: touch bar functionality on BaseWindow Co-authored-by: Shelley Vohr * test: add test for BaseWindow.setTouchBar Co-authored-by: Shelley Vohr --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr --- lib/browser/api/base-window.ts | 6 + lib/browser/api/browser-window.ts | 6 +- lib/browser/api/touch-bar.ts | 6 +- shell/browser/api/electron_api_base_window.cc | 1 - spec/api-touch-bar-spec.ts | 152 +++++++++--------- typings/internal-electron.d.ts | 15 +- 6 files changed, 96 insertions(+), 90 deletions(-) diff --git a/lib/browser/api/base-window.ts b/lib/browser/api/base-window.ts index 71385d21e8ac..8d9c5899da98 100644 --- a/lib/browser/api/base-window.ts +++ b/lib/browser/api/base-window.ts @@ -1,5 +1,7 @@ import { EventEmitter } from 'events'; import type { BaseWindow as TLWT } from 'electron/main'; +import { TouchBar } from 'electron/main'; + const { BaseWindow } = process._linkedBinding('electron_browser_base_window') as { BaseWindow: typeof TLWT }; Object.setPrototypeOf(BaseWindow.prototype, EventEmitter.prototype); @@ -15,6 +17,10 @@ BaseWindow.prototype._init = function (this: TLWT) { } }; +BaseWindow.prototype.setTouchBar = function (touchBar) { + (TouchBar as any)._setOnWindow(touchBar, this); +}; + // Properties Object.defineProperty(BaseWindow.prototype, 'autoHideMenuBar', { diff --git a/lib/browser/api/browser-window.ts b/lib/browser/api/browser-window.ts index 13ec3ef26424..ec6b99e14179 100644 --- a/lib/browser/api/browser-window.ts +++ b/lib/browser/api/browser-window.ts @@ -1,4 +1,4 @@ -import { BaseWindow, WebContents, TouchBar, BrowserView } from 'electron/main'; +import { BaseWindow, WebContents, BrowserView } from 'electron/main'; import type { BrowserWindow as BWT } from 'electron/main'; const { BrowserWindow } = process._linkedBinding('electron_browser_window') as { BrowserWindow: typeof BWT }; @@ -100,10 +100,6 @@ BrowserWindow.fromBrowserView = (browserView: BrowserView) => { return BrowserWindow.fromWebContents(browserView.webContents); }; -BrowserWindow.prototype.setTouchBar = function (touchBar) { - (TouchBar as any)._setOnWindow(touchBar, this); -}; - // Forwarded to webContents: BrowserWindow.prototype.loadURL = function (...args) { diff --git a/lib/browser/api/touch-bar.ts b/lib/browser/api/touch-bar.ts index 8f1c244fd8c9..b9f674f9dfed 100644 --- a/lib/browser/api/touch-bar.ts +++ b/lib/browser/api/touch-bar.ts @@ -284,7 +284,7 @@ const escapeItemSymbol = Symbol('escape item'); class TouchBar extends EventEmitter implements Electron.TouchBar { // Bind a touch bar to a window - static _setOnWindow (touchBar: TouchBar | Electron.TouchBarConstructorOptions['items'], window: Electron.BrowserWindow) { + static _setOnWindow (touchBar: TouchBar | Electron.TouchBarConstructorOptions['items'], window: Electron.BaseWindow) { if (window._touchBar != null) { window._touchBar._removeFromWindow(window); } @@ -383,7 +383,7 @@ class TouchBar extends EventEmitter implements Electron.TouchBar { return this[escapeItemSymbol]; } - _addToWindow (window: Electron.BrowserWindow) { + _addToWindow (window: Electron.BaseWindow) { const { id } = window; // Already added to window @@ -439,7 +439,7 @@ class TouchBar extends EventEmitter implements Electron.TouchBar { escapeItemListener(this.escapeItem); } - _removeFromWindow (window: Electron.BrowserWindow) { + _removeFromWindow (window: Electron.BaseWindow) { const removeListeners = this.windowListeners.get(window.id); if (removeListeners != null) removeListeners(); } diff --git a/shell/browser/api/electron_api_base_window.cc b/shell/browser/api/electron_api_base_window.cc index 3d1a5c292de0..91030ed49194 100644 --- a/shell/browser/api/electron_api_base_window.cc +++ b/shell/browser/api/electron_api_base_window.cc @@ -1251,7 +1251,6 @@ void BaseWindow::BuildPrototype(v8::Isolate* isolate, .SetMethod("setHiddenInMissionControl", &BaseWindow::SetHiddenInMissionControl) #endif - .SetMethod("_setTouchBarItems", &BaseWindow::SetTouchBar) .SetMethod("_refreshTouchBarItem", &BaseWindow::RefreshTouchBarItem) .SetMethod("_setEscapeTouchBarItem", &BaseWindow::SetEscapeTouchBarItem) diff --git a/spec/api-touch-bar-spec.ts b/spec/api-touch-bar-spec.ts index 0de1bbe3b9c2..2d54eef06290 100644 --- a/spec/api-touch-bar-spec.ts +++ b/spec/api-touch-bar-spec.ts @@ -1,5 +1,5 @@ import * as path from 'node:path'; -import { BrowserWindow, TouchBar } from 'electron/main'; +import { BaseWindow, BrowserWindow, TouchBar } from 'electron/main'; import { closeWindow } from './lib/window-helpers'; import { expect } from 'chai'; @@ -47,82 +47,86 @@ describe('TouchBar module', () => { }).to.throw('Cannot add a single instance of TouchBarItem multiple times in a TouchBar'); }); - describe('BrowserWindow behavior', () => { - let window: BrowserWindow; + describe('Window behavior', () => { + for (const WindowType of [BrowserWindow, BaseWindow]) { + describe(`in ${WindowType.name}`, () => { + let window: BaseWindow | BrowserWindow; - beforeEach(() => { - window = new BrowserWindow({ show: false }); - }); + beforeEach(() => { + window = new WindowType({ show: false }); + }); - afterEach(async () => { - window.setTouchBar(null); - await closeWindow(window); - window = null as unknown as BrowserWindow; - }); + afterEach(async () => { + window.setTouchBar(null); + await closeWindow(window); + window = null as unknown as BaseWindow | BrowserWindow; + }); - it('can be added to and removed from a window', () => { - const label = new TouchBarLabel({ label: 'bar' }); - const touchBar = new TouchBar({ - items: [ - new TouchBarButton({ label: 'foo', backgroundColor: '#F00', click: () => { } }), - new TouchBarButton({ - icon: path.join(__dirname, 'fixtures', 'assets', 'logo.png'), - iconPosition: 'right', - click: () => { } - }), - new TouchBarColorPicker({ selectedColor: '#F00', change: () => { } }), - new TouchBarGroup({ items: new TouchBar({ items: [new TouchBarLabel({ label: 'hello' })] }) }), - label, - new TouchBarOtherItemsProxy(), - new TouchBarPopover({ items: new TouchBar({ items: [new TouchBarButton({ label: 'pop' })] }) }), - new TouchBarSlider({ label: 'slide', value: 5, minValue: 2, maxValue: 75, change: () => { } }), - new TouchBarSpacer({ size: 'large' }), - new TouchBarSegmentedControl({ - segmentStyle: 'capsule', - segments: [{ label: 'baz', enabled: false }], - selectedIndex: 5 - }), - new TouchBarSegmentedControl({ segments: [] }), - new TouchBarScrubber({ - items: [{ label: 'foo' }, { label: 'bar' }, { label: 'baz' }], - selectedStyle: 'outline', - mode: 'fixed', - showArrowButtons: true - }) - ] + it('can be added to and removed from a window', () => { + const label = new TouchBarLabel({ label: 'bar' }); + const touchBar = new TouchBar({ + items: [ + new TouchBarButton({ label: 'foo', backgroundColor: '#F00', click: () => { } }), + new TouchBarButton({ + icon: path.join(__dirname, 'fixtures', 'assets', 'logo.png'), + iconPosition: 'right', + click: () => { } + }), + new TouchBarColorPicker({ selectedColor: '#F00', change: () => { } }), + new TouchBarGroup({ items: new TouchBar({ items: [new TouchBarLabel({ label: 'hello' })] }) }), + label, + new TouchBarOtherItemsProxy(), + new TouchBarPopover({ items: new TouchBar({ items: [new TouchBarButton({ label: 'pop' })] }) }), + new TouchBarSlider({ label: 'slide', value: 5, minValue: 2, maxValue: 75, change: () => { } }), + new TouchBarSpacer({ size: 'large' }), + new TouchBarSegmentedControl({ + segmentStyle: 'capsule', + segments: [{ label: 'baz', enabled: false }], + selectedIndex: 5 + }), + new TouchBarSegmentedControl({ segments: [] }), + new TouchBarScrubber({ + items: [{ label: 'foo' }, { label: 'bar' }, { label: 'baz' }], + selectedStyle: 'outline', + mode: 'fixed', + showArrowButtons: true + }) + ] + }); + const escapeButton = new TouchBarButton({ label: 'foo' }); + window.setTouchBar(touchBar); + touchBar.escapeItem = escapeButton; + label.label = 'baz'; + escapeButton.label = 'hello'; + window.setTouchBar(null); + window.setTouchBar(new TouchBar({ items: [new TouchBarLabel({ label: 'two' })] })); + touchBar.escapeItem = null; + }); + + it('calls the callback on the items when a window interaction event fires', (done) => { + const button = new TouchBarButton({ + label: 'bar', + click: () => { + done(); + } + }); + const touchBar = new TouchBar({ items: [button] }); + window.setTouchBar(touchBar); + window.emit('-touch-bar-interaction', {}, (button as any).id); + }); + + it('calls the callback on the escape item when a window interaction event fires', (done) => { + const button = new TouchBarButton({ + label: 'bar', + click: () => { + done(); + } + }); + const touchBar = new TouchBar({ escapeItem: button }); + window.setTouchBar(touchBar); + window.emit('-touch-bar-interaction', {}, (button as any).id); + }); }); - const escapeButton = new TouchBarButton({ label: 'foo' }); - window.setTouchBar(touchBar); - touchBar.escapeItem = escapeButton; - label.label = 'baz'; - escapeButton.label = 'hello'; - window.setTouchBar(null); - window.setTouchBar(new TouchBar({ items: [new TouchBarLabel({ label: 'two' })] })); - touchBar.escapeItem = null; - }); - - it('calls the callback on the items when a window interaction event fires', (done) => { - const button = new TouchBarButton({ - label: 'bar', - click: () => { - done(); - } - }); - const touchBar = new TouchBar({ items: [button] }); - window.setTouchBar(touchBar); - window.emit('-touch-bar-interaction', {}, (button as any).id); - }); - - it('calls the callback on the escape item when a window interaction event fires', (done) => { - const button = new TouchBarButton({ - label: 'bar', - click: () => { - done(); - } - }); - const touchBar = new TouchBar({ escapeItem: button }); - window.setTouchBar(touchBar); - window.emit('-touch-bar-interaction', {}, (button as any).id); - }); + }; }); }); diff --git a/typings/internal-electron.d.ts b/typings/internal-electron.d.ts index 3632b0118ef0..c2ef3a496cba 100644 --- a/typings/internal-electron.d.ts +++ b/typings/internal-electron.d.ts @@ -29,22 +29,23 @@ declare namespace Electron { interface BaseWindow { _init(): void; - } - - interface BrowserWindow { - _init(): void; _touchBar: Electron.TouchBar | null; _setTouchBarItems: (items: TouchBarItemType[]) => void; _setEscapeTouchBarItem: (item: TouchBarItemType | {}) => void; _refreshTouchBarItem: (itemID: string) => void; + on(event: '-touch-bar-interaction', listener: (event: Event, itemID: string, details: any) => void): this; + removeListener(event: '-touch-bar-interaction', listener: (event: Event, itemID: string, details: any) => void): this; + } + + interface BrowserWindow extends BaseWindow { + _init(): void; _getWindowButtonVisibility: () => boolean; _getAlwaysOnTopLevel: () => string; devToolsWebContents: WebContents; frameName: string; + _browserViews: BrowserView[]; on(event: '-touch-bar-interaction', listener: (event: Event, itemID: string, details: any) => void): this; removeListener(event: '-touch-bar-interaction', listener: (event: Event, itemID: string, details: any) => void): this; - - _browserViews: BrowserView[]; } interface BrowserView { @@ -67,7 +68,7 @@ declare namespace Electron { } interface TouchBar { - _removeFromWindow: (win: BrowserWindow) => void; + _removeFromWindow: (win: BaseWindow) => void; } interface WebContents {