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

Jump to bottom

Fix cannot hide terminal find widget when pressing esc #183090

Merged
merged 3 commits into from May 23, 2023
Merged

Fix cannot hide terminal find widget when pressing esc #183090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9385f11
Fix #183087
jeanp413 May 22, 2023
0c2276c
Clean up
Tyriar May 22, 2023
3c7233e
Clean up
jeanp413 May 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 3 additions & 16 deletions src/vs/workbench/contrib/terminal/browser/terminal.ts
View file
Open in desktop
Expand Up @@ -2,7 +2,7 @@
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { Dimension } from 'vs/base/browser/dom';
import { IDimension } from 'vs/base/browser/dom';
import { Orientation } from 'vs/base/browser/ui/splitview/splitview';
import { Color } from 'vs/base/common/color';
import { Event } from 'vs/base/common/event';
Expand Down Expand Up @@ -37,7 +37,7 @@ export const ITerminalInstanceService = createDecorator<ITerminalInstanceService
* been initialized.
*/
export interface ITerminalContribution extends IDisposable {
layout?(xterm: IXtermTerminal & { raw: RawXtermTerminal }): void;
layout?(xterm: IXtermTerminal & { raw: RawXtermTerminal }, dimension: IDimension): void;
xtermReady?(xterm: IXtermTerminal & { raw: RawXtermTerminal }): void;
}

Expand Down Expand Up @@ -434,7 +434,7 @@ export interface ITerminalInstance {
readonly maxRows: number;
readonly fixedCols?: number;
readonly fixedRows?: number;
readonly container?: HTMLElement;
readonly domElement: HTMLElement;
readonly icon?: TerminalIcon;
readonly color?: string;
readonly reconnectionProperties?: IReconnectionProperties;
Expand Down Expand Up @@ -936,25 +936,12 @@ export interface ITerminalInstance {
*/
resetScrollbarVisibility(): void;

/**
* Register a child element to the terminal instance's container.
*/
registerChildElement(element: ITerminalChildElement): IDisposable;

/**
* Gets a terminal contribution by its ID.
*/
getContribution<T extends ITerminalContribution>(id: string): T | null;
}

// NOTE: This interface is very similar to the widget manager internal to TerminalInstance, in the
// future these should probably be consolidated.
export interface ITerminalChildElement {
element: HTMLElement;
layout?(dimension: Dimension): void;
xtermReady?(xterm: IXtermTerminal): void;
}

export const enum XtermTerminalConstants {
SearchHighlightLimit = 1000
}
Expand Down
39 changes: 7 additions & 32 deletions src/vs/workbench/contrib/terminal/browser/terminalInstance.ts
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@ import { ErrorNoTelemetry, onUnexpectedError } from 'vs/base/common/errors';
import { Emitter, Event } from 'vs/base/common/event';
import { KeyCode } from 'vs/base/common/keyCodes';
import { ISeparator, template } from 'vs/base/common/labels';
import { Disposable, DisposableStore, dispose, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
import { Disposable, dispose, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
import { Schemas } from 'vs/base/common/network';
import * as path from 'vs/base/common/path';
import { isMacintosh, isWindows, OperatingSystem, OS } from 'vs/base/common/platform';
Expand Down Expand Up @@ -53,7 +53,7 @@ import { IWorkspaceContextService, IWorkspaceFolder } from 'vs/platform/workspac
import { IWorkspaceTrustRequestService } from 'vs/platform/workspace/common/workspaceTrust';
import { IViewDescriptorService, IViewsService, ViewContainerLocation } from 'vs/workbench/common/views';
import { TaskSettingId } from 'vs/workbench/contrib/tasks/common/tasks';
import { IRequestAddInstanceToGroupEvent, ITerminalChildElement, ITerminalContribution, ITerminalInstance, TerminalDataTransfers } from 'vs/workbench/contrib/terminal/browser/terminal';
import { IRequestAddInstanceToGroupEvent, ITerminalContribution, ITerminalInstance, TerminalDataTransfers } from 'vs/workbench/contrib/terminal/browser/terminal';
import { TerminalLaunchHelpAction } from 'vs/workbench/contrib/terminal/browser/terminalActions';
import { TerminalConfigHelper } from 'vs/workbench/contrib/terminal/browser/terminalConfigHelper';
import { TerminalEditorInput } from 'vs/workbench/contrib/terminal/browser/terminalEditorInput';
Expand Down Expand Up @@ -163,8 +163,8 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
private _title: string = '';
private _titleSource: TitleEventSource = TitleEventSource.Process;
private _container: HTMLElement | undefined;
get container(): HTMLElement | undefined { return this._container; }
private _wrapperElement: (HTMLElement & { xterm?: XTermTerminal });
get domElement(): HTMLElement { return this._wrapperElement; }
Tyriar marked this conversation as resolved.
Show resolved Hide resolved
private _horizontalScrollbar: DomScrollableElement | undefined;
private _terminalFocusContextKey: IContextKey<boolean>;
private _terminalHasFixedWidth: IContextKey<boolean>;
Expand Down Expand Up @@ -335,12 +335,6 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
private readonly _onDidChangeHasChildProcesses = this._register(new Emitter<boolean>());
readonly onDidChangeHasChildProcesses = this._onDidChangeHasChildProcesses.event;

// Internal events
private readonly _onDidAttachToElement = new Emitter<HTMLElement>();
private readonly _onDidAttachToElementEvent = this._onDidAttachToElement.event;
private readonly _onDidLayout = new Emitter<dom.Dimension>();
private readonly _onDidLayoutEvent = this._onDidLayout.event;

constructor(
private readonly _terminalShellTypeContextKey: IContextKey<string>,
private readonly _terminalInRunCommandPicker: IContextKey<boolean>,
Expand Down Expand Up @@ -871,7 +865,6 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
// The container changed, reattach
this._container = container;
this._container.appendChild(this._wrapperElement);
this._onDidAttachToElement.fire(this._container);
this.xterm?.refresh();

setTimeout(() => this._initDragAndDrop(container));
Expand Down Expand Up @@ -1832,17 +1825,16 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
}

this._resize();
this._onDidLayout.fire(dimension);

// Signal the container is ready
this._containerReadyBarrier.open();

// Layout all contributions
for (const contribution of this._contributions.values()) {
if (!this.xterm) {
this._xtermReadyPromise.then(xterm => contribution.layout?.(xterm));
this._xtermReadyPromise.then(xterm => contribution.layout?.(xterm, dimension));
} else {
contribution.layout?.(this.xterm);
contribution.layout?.(this.xterm, dimension);
}
}
}
Expand Down Expand Up @@ -2279,28 +2271,11 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
}

forceScrollbarVisibility(): void {
this._container?.classList.add('force-scrollbar');
this._wrapperElement.classList.add('force-scrollbar');
}

resetScrollbarVisibility(): void {
this._container?.classList.remove('force-scrollbar');
}

registerChildElement(child: ITerminalChildElement): IDisposable {
const store = new DisposableStore();
if (this._container) {
this._container.appendChild(child.element);
} else {
store.add(this._onDidAttachToElementEvent(container => container.appendChild(child.element)));
}
if (this._lastLayoutDimensions) {
child.layout?.(this._lastLayoutDimensions);
}
store.add(this._onDidLayoutEvent(e => child.layout?.(e)));
if (child.xtermReady) {
this._xtermReadyPromise.then(xterm => child.xtermReady!(xterm));
}
return store;
this._wrapperElement.classList.remove('force-scrollbar');
}
}

Expand Down
139 changes: 75 additions & 64 deletions src/vs/workbench/contrib/terminalContrib/find/browser/terminal.find.contribution.ts
View file
Open in desktop
Expand Up @@ -3,65 +3,82 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import { IDimension } from 'vs/base/browser/dom';
import { KeyCode, KeyMod } from 'vs/base/common/keyCodes';
import { Lazy } from 'vs/base/common/lazy';
import { Disposable } from 'vs/base/common/lifecycle';
import { localize } from 'vs/nls';
import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey';
import { IInstantiationService, ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry';
import { findInFilesCommand } from 'vs/workbench/contrib/search/browser/searchActionsFind';
import { ITerminalInstance, ITerminalService } from 'vs/workbench/contrib/terminal/browser/terminal';
import { ITerminalContribution, ITerminalInstance, ITerminalService, IXtermTerminal } from 'vs/workbench/contrib/terminal/browser/terminal';
import { registerActiveInstanceAction } from 'vs/workbench/contrib/terminal/browser/terminalActions';
import { TerminalCommandId } from 'vs/workbench/contrib/terminal/common/terminal';
import { registerTerminalContribution } from 'vs/workbench/contrib/terminal/browser/terminalExtensions';
import { TerminalWidgetManager } from 'vs/workbench/contrib/terminal/browser/widgets/widgetManager';
import { ITerminalProcessManager, TerminalCommandId } from 'vs/workbench/contrib/terminal/common/terminal';
import { TerminalContextKeys } from 'vs/workbench/contrib/terminal/common/terminalContextKey';
import { TerminalFindWidget } from 'vs/workbench/contrib/terminalContrib/find/browser/terminalFindWidget';
import { Terminal as RawXtermTerminal } from 'xterm';

const findWidgets: Map<ITerminalInstance, TerminalFindWidget> = new Map();
class TerminalFindContribution extends Disposable implements ITerminalContribution {
static readonly ID = 'terminal.find';

function getFindWidget(instance: ITerminalInstance | undefined, accessor: ServicesAccessor): TerminalFindWidget | undefined {
if (instance === undefined) {
return undefined;
static get(instance: ITerminalInstance): TerminalFindContribution | null {
return instance.getContribution<TerminalFindContribution>(TerminalFindContribution.ID);
}
let result = findWidgets.get(instance);
if (!result) {
const terminalService = accessor.get(ITerminalService);
const widget = accessor.get(IInstantiationService).createInstance(TerminalFindWidget, instance);

// Track focus and set state so we can force the scroll bar to be visible
let focusState = false;
widget.focusTracker.onDidFocus(() => {
focusState = true;
instance.forceScrollbarVisibility();
terminalService.setActiveInstance(instance);
});
widget.focusTracker.onDidBlur(() => {
focusState = false;
instance.resetScrollbarVisibility();
});

// Attach the find widget and listen for layout
instance.registerChildElement({
element: widget.getDomNode(),
layout: dimension => widget.layout(dimension.width),
xtermReady: xterm => {
xterm.onDidChangeFindResults(() => widget.updateResultCount());
}
});
private _findWidget: Lazy<TerminalFindWidget>;
private _lastLayoutDimensions: IDimension | undefined;

get findWidget(): TerminalFindWidget { return this._findWidget.value; }

// Cache the widget while the instance exists, dispose it when the terminal is disposed
instance.onDisposed(e => {
const focusTerminal = focusState;
widget?.dispose();
findWidgets.delete(e);
if (focusTerminal) {
instance.focus();
constructor(
private readonly _instance: ITerminalInstance,
processManager: ITerminalProcessManager,
widgetManager: TerminalWidgetManager,
@IInstantiationService instantiationService: IInstantiationService,
@ITerminalService terminalService: ITerminalService
) {
super();

this._findWidget = new Lazy(() => {
const findWidget = instantiationService.createInstance(TerminalFindWidget, this._instance);

// Track focus and set state so we can force the scroll bar to be visible
findWidget.focusTracker.onDidFocus(() => {
this._instance.forceScrollbarVisibility();
terminalService.setActiveInstance(this._instance);
});
findWidget.focusTracker.onDidBlur(() => {
this._instance.resetScrollbarVisibility();
});

this._instance.domElement.appendChild(findWidget.getDomNode());
if (this._lastLayoutDimensions) {
findWidget.layout(this._lastLayoutDimensions.width);
}

return findWidget;
});
}

layout(xterm: IXtermTerminal & { raw: RawXtermTerminal }, dimension: IDimension): void {
this._lastLayoutDimensions = dimension;
this._findWidget.rawValue?.layout(dimension.width);
}

xtermReady(xterm: IXtermTerminal & { raw: RawXtermTerminal }): void {
this._register(xterm.onDidChangeFindResults(() => this._findWidget.rawValue?.updateResultCount()));
}

findWidgets.set(instance, widget);
result = widget;
override dispose() {
super.dispose();
this._findWidget.rawValue?.dispose();
}
return result;

}
registerTerminalContribution(TerminalFindContribution.ID, TerminalFindContribution);

registerActiveInstanceAction({
id: TerminalCommandId.FindFocus,
Expand All @@ -72,8 +89,8 @@ registerActiveInstanceAction({
weight: KeybindingWeight.WorkbenchContrib
},
precondition: ContextKeyExpr.or(TerminalContextKeys.processSupported, TerminalContextKeys.terminalHasBeenCreated),
run: (activeInstance, c, accessor) => {
getFindWidget(activeInstance, accessor)?.reveal();
run: (activeInstance) => {
TerminalFindContribution.get(activeInstance)?.findWidget.reveal();
}
});

Expand All @@ -87,8 +104,8 @@ registerActiveInstanceAction({
weight: KeybindingWeight.WorkbenchContrib
},
precondition: ContextKeyExpr.or(TerminalContextKeys.processSupported, TerminalContextKeys.terminalHasBeenCreated),
run: (activeInstance, c, accessor) => {
getFindWidget(activeInstance, accessor)?.hide();
run: (activeInstance) => {
TerminalFindContribution.get(activeInstance)?.findWidget.hide();
}
});

Expand All @@ -102,11 +119,9 @@ registerActiveInstanceAction({
weight: KeybindingWeight.WorkbenchContrib
},
precondition: ContextKeyExpr.or(TerminalContextKeys.processSupported, TerminalContextKeys.terminalHasBeenCreated),
run: (activeInstance, c, accessor) => {
const state = getFindWidget(activeInstance, accessor)?.state;
if (state) {
state.change({ matchCase: !state.isRegex }, false);
}
run: (activeInstance) => {
const state = TerminalFindContribution.get(activeInstance)?.findWidget.state;
state?.change({ matchCase: !state.isRegex }, false);
}
});

Expand All @@ -120,11 +135,9 @@ registerActiveInstanceAction({
weight: KeybindingWeight.WorkbenchContrib
},
precondition: ContextKeyExpr.or(TerminalContextKeys.processSupported, TerminalContextKeys.terminalHasBeenCreated),
run: (activeInstance, c, accessor) => {
const state = getFindWidget(activeInstance, accessor)?.state;
if (state) {
state.change({ matchCase: !state.wholeWord }, false);
}
run: (activeInstance) => {
const state = TerminalFindContribution.get(activeInstance)?.findWidget.state;
state?.change({ matchCase: !state.wholeWord }, false);
}
});

Expand All @@ -138,11 +151,9 @@ registerActiveInstanceAction({
weight: KeybindingWeight.WorkbenchContrib
},
precondition: ContextKeyExpr.or(TerminalContextKeys.processSupported, TerminalContextKeys.terminalHasBeenCreated),
run: (activeInstance, c, accessor) => {
const state = getFindWidget(activeInstance, accessor)?.state;
if (state) {
state.change({ matchCase: !state.matchCase }, false);
}
run: (activeInstance) => {
const state = TerminalFindContribution.get(activeInstance)?.findWidget.state;
state?.change({ matchCase: !state.matchCase }, false);
}
});

Expand All @@ -163,8 +174,8 @@ registerActiveInstanceAction({
}
],
precondition: ContextKeyExpr.or(TerminalContextKeys.processSupported, TerminalContextKeys.terminalHasBeenCreated),
run: (activeInstance, c, accessor) => {
const widget = getFindWidget(activeInstance, accessor);
run: (activeInstance) => {
const widget = TerminalFindContribution.get(activeInstance)?.findWidget;
if (widget) {
widget.show();
widget.find(false);
Expand All @@ -189,8 +200,8 @@ registerActiveInstanceAction({
}
],
precondition: ContextKeyExpr.or(TerminalContextKeys.processSupported, TerminalContextKeys.terminalHasBeenCreated),
run: (activeInstance, c, accessor) => {
const widget = getFindWidget(activeInstance, accessor);
run: (activeInstance) => {
const widget = TerminalFindContribution.get(activeInstance)?.findWidget;
if (widget) {
widget.show();
widget.find(true);
Expand Down