From 65b7b9c78b4a3cb7fb861d879d682044a87a3d1d Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Mon, 17 Oct 2022 11:23:35 -0700 Subject: [PATCH] Find input should not create default controls if they never use them This makes the regex, matchcase, and whole word controls in the find widget optional. Previously they were always being created but then hidden --- src/vs/base/browser/ui/findinput/findInput.ts | 225 +++++++++--------- .../quickinput/browser/quickInputList.ts | 1 + .../contrib/find/notebookFindReplaceWidget.ts | 24 +- 3 files changed, 131 insertions(+), 119 deletions(-) diff --git a/src/vs/base/browser/ui/findinput/findInput.ts b/src/vs/base/browser/ui/findinput/findInput.ts index c164b2fe2a9cb..d9809ddf156ab 100644 --- a/src/vs/base/browser/ui/findinput/findInput.ts +++ b/src/vs/base/browser/ui/findinput/findInput.ts @@ -52,7 +52,7 @@ export class FindInput extends Widget { private placeholder: string; private validation?: IInputValidator; private label: string; - private showCommonFindToggles: boolean; + private readonly showCommonFindToggles: boolean; private fixFocusOnOptionClickEnabled = true; private imeSessionInProgress = false; private additionalTogglesDisposables: DisposableStore = new DisposableStore(); @@ -74,13 +74,13 @@ export class FindInput extends Widget { protected inputValidationErrorBackground?: Color; protected inputValidationErrorForeground?: Color; - protected controls: HTMLDivElement; - protected regex: RegexToggle; - protected wholeWords: WholeWordsToggle; - protected caseSensitive: CaseSensitiveToggle; + protected readonly controls: HTMLDivElement; + protected readonly regex?: RegexToggle; + protected readonly wholeWords?: WholeWordsToggle; + protected readonly caseSensitive?: CaseSensitiveToggle; protected additionalToggles: Toggle[] = []; - public domNode: HTMLElement; - public inputBox: HistoryInputBox; + public readonly domNode: HTMLElement; + public readonly inputBox: HistoryInputBox; private readonly _onDidOptionChange = this._register(new Emitter()); public readonly onDidOptionChange: Event = this._onDidOptionChange.event; @@ -163,103 +163,106 @@ export class FindInput extends Widget { flexibleMaxHeight })); - this.regex = this._register(new RegexToggle({ - appendTitle: appendRegexLabel, - isChecked: false, - inputActiveOptionBorder: this.inputActiveOptionBorder, - inputActiveOptionForeground: this.inputActiveOptionForeground, - inputActiveOptionBackground: this.inputActiveOptionBackground - })); - this._register(this.regex.onChange(viaKeyboard => { - this._onDidOptionChange.fire(viaKeyboard); - if (!viaKeyboard && this.fixFocusOnOptionClickEnabled) { - this.inputBox.focus(); - } - this.validate(); - })); - this._register(this.regex.onKeyDown(e => { - this._onRegexKeyDown.fire(e); - })); + if (this.showCommonFindToggles) { + this.regex = this._register(new RegexToggle({ + appendTitle: appendRegexLabel, + isChecked: false, + inputActiveOptionBorder: this.inputActiveOptionBorder, + inputActiveOptionForeground: this.inputActiveOptionForeground, + inputActiveOptionBackground: this.inputActiveOptionBackground + })); + this._register(this.regex.onChange(viaKeyboard => { + this._onDidOptionChange.fire(viaKeyboard); + if (!viaKeyboard && this.fixFocusOnOptionClickEnabled) { + this.inputBox.focus(); + } + this.validate(); + })); + this._register(this.regex.onKeyDown(e => { + this._onRegexKeyDown.fire(e); + })); - this.wholeWords = this._register(new WholeWordsToggle({ - appendTitle: appendWholeWordsLabel, - isChecked: false, - inputActiveOptionBorder: this.inputActiveOptionBorder, - inputActiveOptionForeground: this.inputActiveOptionForeground, - inputActiveOptionBackground: this.inputActiveOptionBackground - })); - this._register(this.wholeWords.onChange(viaKeyboard => { - this._onDidOptionChange.fire(viaKeyboard); - if (!viaKeyboard && this.fixFocusOnOptionClickEnabled) { - this.inputBox.focus(); - } - this.validate(); - })); + this.wholeWords = this._register(new WholeWordsToggle({ + appendTitle: appendWholeWordsLabel, + isChecked: false, + inputActiveOptionBorder: this.inputActiveOptionBorder, + inputActiveOptionForeground: this.inputActiveOptionForeground, + inputActiveOptionBackground: this.inputActiveOptionBackground + })); + this._register(this.wholeWords.onChange(viaKeyboard => { + this._onDidOptionChange.fire(viaKeyboard); + if (!viaKeyboard && this.fixFocusOnOptionClickEnabled) { + this.inputBox.focus(); + } + this.validate(); + })); - this.caseSensitive = this._register(new CaseSensitiveToggle({ - appendTitle: appendCaseSensitiveLabel, - isChecked: false, - inputActiveOptionBorder: this.inputActiveOptionBorder, - inputActiveOptionForeground: this.inputActiveOptionForeground, - inputActiveOptionBackground: this.inputActiveOptionBackground - })); - this._register(this.caseSensitive.onChange(viaKeyboard => { - this._onDidOptionChange.fire(viaKeyboard); - if (!viaKeyboard && this.fixFocusOnOptionClickEnabled) { - this.inputBox.focus(); - } - this.validate(); - })); - this._register(this.caseSensitive.onKeyDown(e => { - this._onCaseSensitiveKeyDown.fire(e); - })); + this.caseSensitive = this._register(new CaseSensitiveToggle({ + appendTitle: appendCaseSensitiveLabel, + isChecked: false, + inputActiveOptionBorder: this.inputActiveOptionBorder, + inputActiveOptionForeground: this.inputActiveOptionForeground, + inputActiveOptionBackground: this.inputActiveOptionBackground + })); + this._register(this.caseSensitive.onChange(viaKeyboard => { + this._onDidOptionChange.fire(viaKeyboard); + if (!viaKeyboard && this.fixFocusOnOptionClickEnabled) { + this.inputBox.focus(); + } + this.validate(); + })); + this._register(this.caseSensitive.onKeyDown(e => { + this._onCaseSensitiveKeyDown.fire(e); + })); - // Arrow-Key support to navigate between options - const indexes = [this.caseSensitive.domNode, this.wholeWords.domNode, this.regex.domNode]; - this.onkeydown(this.domNode, (event: IKeyboardEvent) => { - if (event.equals(KeyCode.LeftArrow) || event.equals(KeyCode.RightArrow) || event.equals(KeyCode.Escape)) { - const index = indexes.indexOf(document.activeElement); - if (index >= 0) { - let newIndex: number = -1; - if (event.equals(KeyCode.RightArrow)) { - newIndex = (index + 1) % indexes.length; - } else if (event.equals(KeyCode.LeftArrow)) { - if (index === 0) { - newIndex = indexes.length - 1; - } else { - newIndex = index - 1; + // Arrow-Key support to navigate between options + const indexes = [this.caseSensitive.domNode, this.wholeWords.domNode, this.regex.domNode]; + this.onkeydown(this.domNode, (event: IKeyboardEvent) => { + if (event.equals(KeyCode.LeftArrow) || event.equals(KeyCode.RightArrow) || event.equals(KeyCode.Escape)) { + const index = indexes.indexOf(document.activeElement); + if (index >= 0) { + let newIndex: number = -1; + if (event.equals(KeyCode.RightArrow)) { + newIndex = (index + 1) % indexes.length; + } else if (event.equals(KeyCode.LeftArrow)) { + if (index === 0) { + newIndex = indexes.length - 1; + } else { + newIndex = index - 1; + } } - } - if (event.equals(KeyCode.Escape)) { - indexes[index].blur(); - this.inputBox.focus(); - } else if (newIndex >= 0) { - indexes[newIndex].focus(); - } + if (event.equals(KeyCode.Escape)) { + indexes[index].blur(); + this.inputBox.focus(); + } else if (newIndex >= 0) { + indexes[newIndex].focus(); + } - dom.EventHelper.stop(event, true); + dom.EventHelper.stop(event, true); + } } - } - }); - + }); + } this.controls = document.createElement('div'); this.controls.className = 'controls'; this.controls.style.display = this.showCommonFindToggles ? 'block' : 'none'; - this.controls.appendChild(this.caseSensitive.domNode); - this.controls.appendChild(this.wholeWords.domNode); - this.controls.appendChild(this.regex.domNode); - - if (!this.showCommonFindToggles) { - this.caseSensitive.domNode.style.display = 'none'; - this.wholeWords.domNode.style.display = 'none'; - this.regex.domNode.style.display = 'none'; + if (this.caseSensitive) { + this.controls.append(this.caseSensitive.domNode); + } + if (this.wholeWords) { + this.controls.appendChild(this.wholeWords.domNode); + } + if (this.regex) { + this.controls.appendChild(this.regex.domNode); } this.setAdditionalToggles(options?.additionalToggles); - this.domNode.appendChild(this.controls); + if (this.controls) { + this.domNode.appendChild(this.controls); + } parent?.appendChild(this.domNode); @@ -288,9 +291,9 @@ export class FindInput extends Widget { public enable(): void { this.domNode.classList.remove('disabled'); this.inputBox.enable(); - this.regex.enable(); - this.wholeWords.enable(); - this.caseSensitive.enable(); + this.regex?.enable(); + this.wholeWords?.enable(); + this.caseSensitive?.enable(); for (const toggle of this.additionalToggles) { toggle.enable(); @@ -300,9 +303,9 @@ export class FindInput extends Widget { public disable(): void { this.domNode.classList.add('disabled'); this.inputBox.disable(); - this.regex.disable(); - this.wholeWords.disable(); - this.caseSensitive.disable(); + this.regex?.disable(); + this.wholeWords?.disable(); + this.caseSensitive?.disable(); for (const toggle of this.additionalToggles) { toggle.disable(); @@ -348,7 +351,7 @@ export class FindInput extends Widget { } this.inputBox.paddingRight = - (this.showCommonFindToggles ? this.caseSensitive.width() + this.wholeWords.width() + this.regex.width() : 0) + ((this.caseSensitive?.width() ?? 0) + (this.wholeWords?.width() ?? 0) + (this.regex?.width() ?? 0)) + this.additionalToggles.reduce((r, t) => r + t.width(), 0); } @@ -400,9 +403,9 @@ export class FindInput extends Widget { inputActiveOptionForeground: this.inputActiveOptionForeground, inputActiveOptionBackground: this.inputActiveOptionBackground, }; - this.regex.style(toggleStyles); - this.wholeWords.style(toggleStyles); - this.caseSensitive.style(toggleStyles); + this.regex?.style(toggleStyles); + this.wholeWords?.style(toggleStyles); + this.caseSensitive?.style(toggleStyles); for (const toggle of this.additionalToggles) { toggle.style(toggleStyles); @@ -435,36 +438,42 @@ export class FindInput extends Widget { } public getCaseSensitive(): boolean { - return this.caseSensitive.checked; + return this.caseSensitive?.checked ?? false; } public setCaseSensitive(value: boolean): void { - this.caseSensitive.checked = value; + if (this.caseSensitive) { + this.caseSensitive.checked = value; + } } public getWholeWords(): boolean { - return this.wholeWords.checked; + return this.wholeWords?.checked ?? false; } public setWholeWords(value: boolean): void { - this.wholeWords.checked = value; + if (this.wholeWords) { + this.wholeWords.checked = value; + } } public getRegex(): boolean { - return this.regex.checked; + return this.regex?.checked ?? false; } public setRegex(value: boolean): void { - this.regex.checked = value; - this.validate(); + if (this.regex) { + this.regex.checked = value; + this.validate(); + } } public focusOnCaseSensitive(): void { - this.caseSensitive.focus(); + this.caseSensitive?.focus(); } public focusOnRegex(): void { - this.regex.focus(); + this.regex?.focus(); } private _lastHighlightFindOptions: number = 0; diff --git a/src/vs/base/parts/quickinput/browser/quickInputList.ts b/src/vs/base/parts/quickinput/browser/quickInputList.ts index 515bae093a6e1..82f4f05cf0fd4 100644 --- a/src/vs/base/parts/quickinput/browser/quickInputList.ts +++ b/src/vs/base/parts/quickinput/browser/quickInputList.ts @@ -309,6 +309,7 @@ export class QuickInputList { ) { this.id = id; this.container = dom.append(this.parent, $('.quick-input-list')); + const delegate = new ListElementDelegate(); const accessibilityProvider = new QuickInputAccessibilityProvider(); this.list = options.createList('QuickInput', this.container, delegate, [new ListElementRenderer()], { diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/find/notebookFindReplaceWidget.ts b/src/vs/workbench/contrib/notebook/browser/contrib/find/notebookFindReplaceWidget.ts index 36c5785aa7fcf..736a6f2eb1ecc 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/find/notebookFindReplaceWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/find/notebookFindReplaceWidget.ts @@ -181,7 +181,7 @@ class NotebookFindInput extends FindInput { } })); - this.inputBox.paddingRight = this.caseSensitive.width() + this.wholeWords.width() + this.regex.width() + this.getFilterWidth(); + this.inputBox.paddingRight = (this.caseSensitive?.width() ?? 0) + (this.wholeWords?.width() ?? 0) + (this.regex?.width() ?? 0) + this.getFilterWidth(); } private getFilterWidth() { @@ -203,22 +203,24 @@ class NotebookFindInput extends FindInput { override setEnabled(enabled: boolean) { super.setEnabled(enabled); if (enabled && !this._filterChecked) { - this.regex.enable(); + this.regex?.enable(); } else { - this.regex.disable(); + this.regex?.disable(); } } updateFilterState(changed: boolean) { this._filterChecked = changed; - if (this._filterChecked) { - this.regex.disable(); - this.regex.domNode.tabIndex = -1; - this.regex.domNode.classList.toggle('disabled', true); - } else { - this.regex.enable(); - this.regex.domNode.tabIndex = 0; - this.regex.domNode.classList.toggle('disabled', false); + if (this.regex) { + if (this._filterChecked) { + this.regex.disable(); + this.regex.domNode.tabIndex = -1; + this.regex.domNode.classList.toggle('disabled', true); + } else { + this.regex.enable(); + this.regex.domNode.tabIndex = 0; + this.regex.domNode.classList.toggle('disabled', false); + } } this.applyStyles(); }