Skip to content

Commit 4d0b3d0

Browse files
author
Benjamin Pasero
committed
debt week code cleanup
- avoid public modifier - use Disposable where applicable - fix some event handler leaks - clean up some TODO@ben
1 parent cc2a164 commit 4d0b3d0

162 files changed

Lines changed: 2468 additions & 3026 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎build/lib/i18n.resources.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@
7878
"name": "vs/workbench/parts/logs",
7979
"project": "vscode-workbench"
8080
},
81+
{
82+
"name": "vs/workbench/parts/navigation",
83+
"project": "vscode-workbench"
84+
},
8185
{
8286
"name": "vs/workbench/parts/output",
8387
"project": "vscode-workbench"

‎src/vs/base/browser/builder.ts‎

Lines changed: 123 additions & 123 deletions
Large diffs are not rendered by default.

‎src/vs/base/browser/dnd.ts‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,20 @@
55

66
'use strict';
77

8-
import { IDisposable, dispose } from 'vs/base/common/lifecycle';
8+
import { Disposable } from 'vs/base/common/lifecycle';
99
import { addDisposableListener } from 'vs/base/browser/dom';
1010

1111
/**
1212
* A helper that will execute a provided function when the provided HTMLElement receives
1313
* dragover event for 800ms. If the drag is aborted before, the callback will not be triggered.
1414
*/
15-
export class DelayedDragHandler {
16-
private toDispose: IDisposable[] = [];
15+
export class DelayedDragHandler extends Disposable {
1716
private timeout: number;
1817

1918
constructor(container: HTMLElement, callback: () => void) {
20-
this.toDispose.push(addDisposableListener(container, 'dragover', () => {
19+
super();
20+
21+
this._register(addDisposableListener(container, 'dragover', () => {
2122
if (!this.timeout) {
2223
this.timeout = setTimeout(() => {
2324
callback();
@@ -28,7 +29,7 @@ export class DelayedDragHandler {
2829
}));
2930

3031
['dragleave', 'drop', 'dragend'].forEach(type => {
31-
this.toDispose.push(addDisposableListener(container, type, () => {
32+
this._register(addDisposableListener(container, type, () => {
3233
this.clearDragTimeout();
3334
}));
3435
});
@@ -41,8 +42,9 @@ export class DelayedDragHandler {
4142
}
4243
}
4344

44-
public dispose(): void {
45-
this.toDispose = dispose(this.toDispose);
45+
dispose(): void {
46+
super.dispose();
47+
4648
this.clearDragTimeout();
4749
}
4850
}

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

Lines changed: 16 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { KeyCode } from 'vs/base/common/keyCodes';
1313
import { Color } from 'vs/base/common/color';
1414
import { mixin } from 'vs/base/common/objects';
1515
import { Event as BaseEvent, Emitter } from 'vs/base/common/event';
16-
import { dispose, IDisposable } from 'vs/base/common/lifecycle';
16+
import { Disposable } from 'vs/base/common/lifecycle';
1717
import { Gesture, EventType } from 'vs/base/browser/touch';
1818

1919
export interface IButtonOptions extends IButtonStyles {
@@ -33,7 +33,7 @@ const defaultOptions: IButtonStyles = {
3333
buttonForeground: Color.white
3434
};
3535

36-
export class Button {
36+
export class Button extends Disposable {
3737

3838
private $el: Builder;
3939
private options: IButtonOptions;
@@ -43,12 +43,14 @@ export class Button {
4343
private buttonForeground: Color;
4444
private buttonBorder: Color;
4545

46-
private _onDidClick = new Emitter<any>();
47-
readonly onDidClick: BaseEvent<Event> = this._onDidClick.event;
46+
private _onDidClick = this._register(new Emitter<any>());
47+
get onDidClick(): BaseEvent<Event> { return this._onDidClick.event; }
4848

4949
private focusTracker: DOM.IFocusTracker;
5050

5151
constructor(container: HTMLElement, options?: IButtonOptions) {
52+
super();
53+
5254
this.options = options || Object.create(null);
5355
mixin(this.options, defaultOptions, false);
5456

@@ -57,10 +59,10 @@ export class Button {
5759
this.buttonForeground = this.options.buttonForeground;
5860
this.buttonBorder = this.options.buttonBorder;
5961

60-
this.$el = $('a.monaco-button').attr({
62+
this.$el = this._register($('a.monaco-button').attr({
6163
'tabIndex': '0',
6264
'role': 'button'
63-
}).appendTo(container);
65+
}).appendTo(container));
6466

6567
Gesture.addTarget(this.$el.getHTMLElement());
6668

@@ -74,7 +76,7 @@ export class Button {
7476
});
7577

7678
this.$el.on(DOM.EventType.KEY_DOWN, e => {
77-
let event = new StandardKeyboardEvent(e as KeyboardEvent);
79+
const event = new StandardKeyboardEvent(e as KeyboardEvent);
7880
let eventHandled = false;
7981
if (this.enabled && event.equals(KeyCode.Enter) || event.equals(KeyCode.Space)) {
8082
this._onDidClick.fire(e);
@@ -100,9 +102,9 @@ export class Button {
100102
});
101103

102104
// Also set hover background when button is focused for feedback
103-
this.focusTracker = DOM.trackFocus(this.$el.getHTMLElement());
104-
this.focusTracker.onDidFocus(() => this.setHoverBackground());
105-
this.focusTracker.onDidBlur(() => this.applyStyles()); // restore standard styles
105+
this.focusTracker = this._register(DOM.trackFocus(this.$el.getHTMLElement()));
106+
this._register(this.focusTracker.onDidFocus(() => this.setHoverBackground()));
107+
this._register(this.focusTracker.onDidBlur(() => this.applyStyles())); // restore standard styles
106108

107109
this.applyStyles();
108110
}
@@ -177,27 +179,13 @@ export class Button {
177179
focus(): void {
178180
this.$el.domFocus();
179181
}
180-
181-
dispose(): void {
182-
if (this.$el) {
183-
this.$el.dispose();
184-
this.$el = null;
185-
186-
this.focusTracker.dispose();
187-
this.focusTracker = null;
188-
}
189-
190-
this._onDidClick.dispose();
191-
}
192182
}
193183

194-
export class ButtonGroup {
195-
private _buttons: Button[];
196-
private toDispose: IDisposable[];
184+
export class ButtonGroup extends Disposable {
185+
private _buttons: Button[] = [];
197186

198187
constructor(container: HTMLElement, count: number, options?: IButtonOptions) {
199-
this._buttons = [];
200-
this.toDispose = [];
188+
super();
201189

202190
this.create(container, count, options);
203191
}
@@ -208,9 +196,8 @@ export class ButtonGroup {
208196

209197
private create(container: HTMLElement, count: number, options?: IButtonOptions): void {
210198
for (let index = 0; index < count; index++) {
211-
const button = new Button(container, options);
199+
const button = this._register(new Button(container, options));
212200
this._buttons.push(button);
213-
this.toDispose.push(button);
214201

215202
// Implement keyboard access in buttons if there are multiple
216203
if (count > 1) {
@@ -236,8 +223,4 @@ export class ButtonGroup {
236223
}
237224
}
238225
}
239-
240-
dispose(): void {
241-
this.toDispose = dispose(this.toDispose);
242-
}
243226
}

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

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

66
'use strict';
77

8+
import 'vs/css!./checkbox';
89
import * as DOM from 'vs/base/browser/dom';
910
import { IKeyboardEvent } from 'vs/base/browser/keyboardEvent';
1011
import { Widget } from 'vs/base/browser/ui/widget';
1112
import { Color } from 'vs/base/common/color';
1213
import { Emitter, Event } from 'vs/base/common/event';
1314
import { KeyCode } from 'vs/base/common/keyCodes';
1415
import * as objects from 'vs/base/common/objects';
15-
import 'vs/css!./checkbox';
1616

1717
export interface ICheckboxOpts extends ICheckboxStyles {
1818
readonly actionClassName: string;
@@ -31,18 +31,19 @@ const defaultOpts = {
3131
export class Checkbox extends Widget {
3232

3333
private readonly _onChange = this._register(new Emitter<boolean>());
34-
public readonly onChange: Event<boolean /* via keyboard */> = this._onChange.event;
34+
get onChange(): Event<boolean /* via keyboard */> { return this._onChange.event; }
3535

3636
private readonly _onKeyDown = this._register(new Emitter<IKeyboardEvent>());
37-
public readonly onKeyDown: Event<IKeyboardEvent> = this._onKeyDown.event;
37+
get onKeyDown(): Event<IKeyboardEvent> { return this._onKeyDown.event; }
3838

3939
private readonly _opts: ICheckboxOpts;
40-
public readonly domNode: HTMLElement;
40+
readonly domNode: HTMLElement;
4141

4242
private _checked: boolean;
4343

4444
constructor(opts: ICheckboxOpts) {
4545
super();
46+
4647
this._opts = objects.deepClone(opts);
4748
objects.mixin(this._opts, defaultOpts, false);
4849
this._checked = this._opts.isChecked;
@@ -75,19 +76,19 @@ export class Checkbox extends Widget {
7576
});
7677
}
7778

78-
public get enabled(): boolean {
79+
get enabled(): boolean {
7980
return this.domNode.getAttribute('aria-disabled') !== 'true';
8081
}
8182

82-
public focus(): void {
83+
focus(): void {
8384
this.domNode.focus();
8485
}
8586

86-
public get checked(): boolean {
87+
get checked(): boolean {
8788
return this._checked;
8889
}
8990

90-
public set checked(newIsChecked: boolean) {
91+
set checked(newIsChecked: boolean) {
9192
this._checked = newIsChecked;
9293
this.domNode.setAttribute('aria-checked', String(this._checked));
9394
if (this._checked) {
@@ -99,11 +100,11 @@ export class Checkbox extends Widget {
99100
this.applyStyles();
100101
}
101102

102-
public width(): number {
103+
width(): number {
103104
return 2 /*marginleft*/ + 2 /*border*/ + 2 /*padding*/ + 16 /* icon width */;
104105
}
105106

106-
public style(styles: ICheckboxStyles): void {
107+
style(styles: ICheckboxStyles): void {
107108
if (styles.inputActiveOptionBorder) {
108109
this._opts.inputActiveOptionBorder = styles.inputActiveOptionBorder;
109110
}
@@ -116,12 +117,12 @@ export class Checkbox extends Widget {
116117
}
117118
}
118119

119-
public enable(): void {
120+
enable(): void {
120121
this.domNode.tabIndex = 0;
121122
this.domNode.setAttribute('aria-disabled', String(false));
122123
}
123124

124-
public disable(): void {
125+
disable(): void {
125126
DOM.removeTabIndexAndUpdateFocus(this.domNode);
126127
this.domNode.setAttribute('aria-disabled', String(true));
127128
}

0 commit comments

Comments
 (0)