Skip to content

Commit e0fbb42

Browse files
roblourensroblourens
committed
Change custom Checkbox to expose an onChange Event instead of taking an onChange callback in options.
Otherwise it's impossible to reference the checkbox from inside the callback while constructing a Checkbox
1 parent bf4f7de commit e0fbb42

9 files changed

Lines changed: 82 additions & 84 deletions

File tree

‎src/vs/base/browser/ui/checkbox/checkbox.ts‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,19 @@
55

66
'use strict';
77

8-
import 'vs/css!./checkbox';
9-
108
import * as DOM from 'vs/base/browser/dom';
11-
import * as objects from 'vs/base/common/objects';
12-
import { KeyCode } from 'vs/base/common/keyCodes';
13-
import { Widget } from 'vs/base/browser/ui/widget';
149
import { IKeyboardEvent } from 'vs/base/browser/keyboardEvent';
10+
import { Widget } from 'vs/base/browser/ui/widget';
1511
import { Color } from 'vs/base/common/color';
12+
import { Emitter, Event } from 'vs/base/common/event';
13+
import { KeyCode } from 'vs/base/common/keyCodes';
14+
import * as objects from 'vs/base/common/objects';
15+
import 'vs/css!./checkbox';
1616

1717
export interface ICheckboxOpts extends ICheckboxStyles {
1818
readonly actionClassName: string;
1919
readonly title: string;
2020
readonly isChecked: boolean;
21-
readonly onChange: (viaKeyboard: boolean) => void;
22-
readonly onKeyDown?: (e: IKeyboardEvent) => void;
2321
}
2422

2523
export interface ICheckboxStyles {
@@ -32,6 +30,12 @@ const defaultOpts = {
3230

3331
export class Checkbox extends Widget {
3432

33+
private readonly _onChange = this._register(new Emitter<boolean>());
34+
public readonly onChange: Event<boolean /* via keyboard */> = this._onChange.event;
35+
36+
private readonly _onKeyDown = this._register(new Emitter<IKeyboardEvent>());
37+
public readonly onKeyDown: Event<IKeyboardEvent> = this._onKeyDown.event;
38+
3539
private readonly _opts: ICheckboxOpts;
3640
public readonly domNode: HTMLElement;
3741

@@ -55,21 +59,19 @@ export class Checkbox extends Widget {
5559

5660
this.onclick(this.domNode, (ev) => {
5761
this.checked = !this._checked;
58-
this._opts.onChange(false);
62+
this._onChange.fire(false);
5963
ev.preventDefault();
6064
});
6165

6266
this.onkeydown(this.domNode, (keyboardEvent) => {
6367
if (keyboardEvent.keyCode === KeyCode.Space || keyboardEvent.keyCode === KeyCode.Enter) {
6468
this.checked = !this._checked;
65-
this._opts.onChange(true);
69+
this._onChange.fire(true);
6670
keyboardEvent.preventDefault();
6771
return;
6872
}
6973

70-
if (this._opts.onKeyDown) {
71-
this._opts.onKeyDown(keyboardEvent);
72-
}
74+
this._onKeyDown.fire(keyboardEvent);
7375
});
7476
}
7577

‎src/vs/base/browser/ui/findinput/findInput.ts‎

Lines changed: 32 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -293,48 +293,50 @@ export class FindInput extends Widget {
293293
this.regex = this._register(new RegexCheckbox({
294294
appendTitle: appendRegexLabel,
295295
isChecked: false,
296-
onChange: (viaKeyboard) => {
297-
this._onDidOptionChange.fire(viaKeyboard);
298-
if (!viaKeyboard) {
299-
this.inputBox.focus();
300-
}
301-
this.setInputWidth();
302-
this.validate();
303-
},
304-
onKeyDown: (e) => {
305-
this._onRegexKeyDown.fire(e);
306-
},
307296
inputActiveOptionBorder: this.inputActiveOptionBorder
308297
}));
298+
this._register(this.regex.onChange(viaKeyboard => {
299+
this._onDidOptionChange.fire(viaKeyboard);
300+
if (!viaKeyboard) {
301+
this.inputBox.focus();
302+
}
303+
this.setInputWidth();
304+
this.validate();
305+
}));
306+
this._register(this.regex.onKeyDown(e => {
307+
this._onRegexKeyDown.fire(e);
308+
}));
309+
309310
this.wholeWords = this._register(new WholeWordsCheckbox({
310311
appendTitle: appendWholeWordsLabel,
311312
isChecked: false,
312-
onChange: (viaKeyboard) => {
313-
this._onDidOptionChange.fire(viaKeyboard);
314-
if (!viaKeyboard) {
315-
this.inputBox.focus();
316-
}
317-
this.setInputWidth();
318-
this.validate();
319-
},
320313
inputActiveOptionBorder: this.inputActiveOptionBorder
321314
}));
315+
this._register(this.wholeWords.onChange(viaKeyboard => {
316+
this._onDidOptionChange.fire(viaKeyboard);
317+
if (!viaKeyboard) {
318+
this.inputBox.focus();
319+
}
320+
this.setInputWidth();
321+
this.validate();
322+
}));
323+
322324
this.caseSensitive = this._register(new CaseSensitiveCheckbox({
323325
appendTitle: appendCaseSensitiveLabel,
324326
isChecked: false,
325-
onChange: (viaKeyboard) => {
326-
this._onDidOptionChange.fire(viaKeyboard);
327-
if (!viaKeyboard) {
328-
this.inputBox.focus();
329-
}
330-
this.setInputWidth();
331-
this.validate();
332-
},
333-
onKeyDown: (e) => {
334-
this._onCaseSensitiveKeyDown.fire(e);
335-
},
336327
inputActiveOptionBorder: this.inputActiveOptionBorder
337328
}));
329+
this._register(this.caseSensitive.onChange(viaKeyboard => {
330+
this._onDidOptionChange.fire(viaKeyboard);
331+
if (!viaKeyboard) {
332+
this.inputBox.focus();
333+
}
334+
this.setInputWidth();
335+
this.validate();
336+
}));
337+
this._register(this.caseSensitive.onKeyDown(e => {
338+
this._onCaseSensitiveKeyDown.fire(e);
339+
}));
338340

339341
// Arrow-Key support to navigate between options
340342
let indexes = [this.caseSensitive.domNode, this.wholeWords.domNode, this.regex.domNode];

‎src/vs/base/browser/ui/findinput/findInputCheckboxes.ts‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,18 +4,14 @@
44
*--------------------------------------------------------------------------------------------*/
55
'use strict';
66

7-
import 'vs/css!./findInputCheckboxes';
8-
9-
import * as nls from 'vs/nls';
107
import { Checkbox } from 'vs/base/browser/ui/checkbox/checkbox';
11-
import { IKeyboardEvent } from 'vs/base/browser/keyboardEvent';
128
import { Color } from 'vs/base/common/color';
9+
import 'vs/css!./findInputCheckboxes';
10+
import * as nls from 'vs/nls';
1311

1412
export interface IFindInputCheckboxOpts {
1513
readonly appendTitle: string;
1614
readonly isChecked: boolean;
17-
readonly onChange: (viaKeyboard: boolean) => void;
18-
readonly onKeyDown?: (e: IKeyboardEvent) => void;
1915
readonly inputActiveOptionBorder?: Color;
2016
}
2117

@@ -29,8 +25,6 @@ export class CaseSensitiveCheckbox extends Checkbox {
2925
actionClassName: 'monaco-case-sensitive',
3026
title: NLS_CASE_SENSITIVE_CHECKBOX_LABEL + opts.appendTitle,
3127
isChecked: opts.isChecked,
32-
onChange: opts.onChange,
33-
onKeyDown: opts.onKeyDown,
3428
inputActiveOptionBorder: opts.inputActiveOptionBorder
3529
});
3630
}
@@ -42,8 +36,6 @@ export class WholeWordsCheckbox extends Checkbox {
4236
actionClassName: 'monaco-whole-word',
4337
title: NLS_WHOLE_WORD_CHECKBOX_LABEL + opts.appendTitle,
4438
isChecked: opts.isChecked,
45-
onChange: opts.onChange,
46-
onKeyDown: opts.onKeyDown,
4739
inputActiveOptionBorder: opts.inputActiveOptionBorder
4840
});
4941
}
@@ -55,8 +47,6 @@ export class RegexCheckbox extends Checkbox {
5547
actionClassName: 'monaco-regex',
5648
title: NLS_REGEX_CHECKBOX_LABEL + opts.appendTitle,
5749
isChecked: opts.isChecked,
58-
onChange: opts.onChange,
59-
onKeyDown: opts.onKeyDown,
6050
inputActiveOptionBorder: opts.inputActiveOptionBorder
6151
});
6252
}

‎src/vs/editor/contrib/find/findOptionsWidget.ts‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -53,38 +53,38 @@ export class FindOptionsWidget extends Widget implements IOverlayWidget {
5353
this.caseSensitive = this._register(new CaseSensitiveCheckbox({
5454
appendTitle: this._keybindingLabelFor(FIND_IDS.ToggleCaseSensitiveCommand),
5555
isChecked: this._state.matchCase,
56-
onChange: (viaKeyboard) => {
57-
this._state.change({
58-
matchCase: this.caseSensitive.checked
59-
}, false);
60-
},
6156
inputActiveOptionBorder: inputActiveOptionBorderColor
6257
}));
6358
this._domNode.appendChild(this.caseSensitive.domNode);
59+
this._register(this.caseSensitive.onChange(() => {
60+
this._state.change({
61+
matchCase: this.caseSensitive.checked
62+
}, false);
63+
}));
6464

6565
this.wholeWords = this._register(new WholeWordsCheckbox({
6666
appendTitle: this._keybindingLabelFor(FIND_IDS.ToggleWholeWordCommand),
6767
isChecked: this._state.wholeWord,
68-
onChange: (viaKeyboard) => {
69-
this._state.change({
70-
wholeWord: this.wholeWords.checked
71-
}, false);
72-
},
7368
inputActiveOptionBorder: inputActiveOptionBorderColor
7469
}));
7570
this._domNode.appendChild(this.wholeWords.domNode);
71+
this._register(this.wholeWords.onChange(() => {
72+
this._state.change({
73+
wholeWord: this.wholeWords.checked
74+
}, false);
75+
}));
7676

7777
this.regex = this._register(new RegexCheckbox({
7878
appendTitle: this._keybindingLabelFor(FIND_IDS.ToggleRegexCommand),
7979
isChecked: this._state.isRegex,
80-
onChange: (viaKeyboard) => {
81-
this._state.change({
82-
isRegex: this.regex.checked
83-
}, false);
84-
},
8580
inputActiveOptionBorder: inputActiveOptionBorderColor
8681
}));
8782
this._domNode.appendChild(this.regex.domNode);
83+
this._register(this.regex.onChange(() => {
84+
this._state.change({
85+
isRegex: this.regex.checked
86+
}, false);
87+
}));
8888

8989
this._editor.addOverlayWidget(this);
9090

‎src/vs/workbench/parts/markers/electron-browser/markersPanelActions.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -188,15 +188,16 @@ export class MarkersFilterActionItem extends BaseActionItem {
188188
}
189189

190190
private createFilesExcludeCheckbox(container: HTMLElement): void {
191-
this.filesExcludeFilter = new Checkbox({
191+
this.filesExcludeFilter = this._register(new Checkbox({
192192
actionClassName: 'markers-panel-filter-filesExclude',
193193
title: this.itemOptions.useFilesExclude ? Messages.MARKERS_PANEL_ACTION_TOOLTIP_DO_NOT_USE_FILES_EXCLUDE : Messages.MARKERS_PANEL_ACTION_TOOLTIP_USE_FILES_EXCLUDE,
194-
isChecked: this.itemOptions.useFilesExclude,
195-
onChange: () => {
196-
this.filesExcludeFilter.domNode.title = this.filesExcludeFilter.checked ? Messages.MARKERS_PANEL_ACTION_TOOLTIP_DO_NOT_USE_FILES_EXCLUDE : Messages.MARKERS_PANEL_ACTION_TOOLTIP_USE_FILES_EXCLUDE;
197-
this._onDidChange.fire();
198-
}
199-
});
194+
isChecked: this.itemOptions.useFilesExclude
195+
}));
196+
this._register(this.filesExcludeFilter.onChange(() => {
197+
this.filesExcludeFilter.domNode.title = this.filesExcludeFilter.checked ? Messages.MARKERS_PANEL_ACTION_TOOLTIP_DO_NOT_USE_FILES_EXCLUDE : Messages.MARKERS_PANEL_ACTION_TOOLTIP_USE_FILES_EXCLUDE;
198+
this._onDidChange.fire();
199+
}));
200+
200201
this._register(attachCheckboxStyler(this.filesExcludeFilter, this.themeService));
201202
container.appendChild(this.filesExcludeFilter.domNode);
202203
}

‎src/vs/workbench/parts/preferences/browser/keybindingsEditor.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,9 +296,10 @@ export class KeybindingsEditor extends BaseEditor implements IKeybindingsEditor
296296
this.sortByPrecedence = this._register(new Checkbox({
297297
actionClassName: 'sort-by-precedence',
298298
isChecked: false,
299-
onChange: () => this.renderKeybindingsEntries(false),
300299
title: localize('sortByPrecedene', "Sort by Precedence")
301300
}));
301+
this._register(
302+
this.sortByPrecedence.onChange(() => this.renderKeybindingsEntries(false)));
302303
searchContainer.appendChild(this.sortByPrecedence.domNode);
303304

304305
this.createOpenKeybindingsElement(this.headerContainer);

‎src/vs/workbench/parts/preferences/browser/media/check-inverse.svg‎

Lines changed: 1 addition & 0 deletions
Loading

‎src/vs/workbench/parts/preferences/browser/media/check.svg‎

Lines changed: 1 addition & 0 deletions
Loading

‎src/vs/workbench/parts/search/browser/patternInputWidget.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -215,17 +215,17 @@ export class ExcludePatternInputWidget extends PatternInputWidget {
215215
}
216216

217217
protected renderSubcontrols(controlsDiv: HTMLDivElement): void {
218-
this.useExcludesAndIgnoreFilesBox = new Checkbox({
218+
this.useExcludesAndIgnoreFilesBox = this._register(new Checkbox({
219219
actionClassName: 'useExcludesAndIgnoreFiles',
220220
title: nls.localize('useExcludesAndIgnoreFilesDescription', "Use Exclude Settings and Ignore Files"),
221221
isChecked: true,
222-
onChange: (viaKeyboard) => {
223-
this.onOptionChange(null);
224-
if (!viaKeyboard) {
225-
this.inputBox.focus();
226-
}
222+
}));
223+
this._register(this.useExcludesAndIgnoreFilesBox.onChange(viaKeyboard => {
224+
this.onOptionChange(null);
225+
if (!viaKeyboard) {
226+
this.inputBox.focus();
227227
}
228-
});
228+
}));
229229
this._register(attachCheckboxStyler(this.useExcludesAndIgnoreFilesBox, this.themeService));
230230

231231
controlsDiv.appendChild(this.useExcludesAndIgnoreFilesBox.domNode);

0 commit comments

Comments
 (0)