Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 674f0a0

Browse files
author
Akos Kitta
committed
fix: debug widget layout updates
Use customized `PanelLayout#removeWidget` and `PanelLayout#insertWidget` logic for the layout updates. The customized functions ensure no side effect if adding/removing the widget to/from the layout but it's already present/absent. Unlike the default [`PanelLayout#removeWidget`](https://github.com/phosphorjs/phosphor/blob/9f5e11025b62d2c4a6fb59e2681ae1ed323dcde4/packages/widgets/src/panellayout.ts#L154-L156) behavior, do not try to remove the widget if it's not present (has a negative index). Otherwise, required widgets might be removed based on the default [`ArrayExt#removeAt`](https://github.com/phosphorjs/phosphor/blob/9f5e11025b62d2c4a6fb59e2681ae1ed323dcde4/packages/algorithm/src/array.ts#L1075-L1077) behavior. Closes: #2354 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
1 parent 74c5801 commit 674f0a0

File tree

3 files changed

+145
-11
lines changed

3 files changed

+145
-11
lines changed

‎arduino-ide-extension/src/browser/theia/debug/debug-widget.ts‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
1-
import {
2-
codicon,
3-
PanelLayout,
4-
Widget,
5-
} from '@theia/core/lib/browser/widgets/widget';
1+
import { codicon } from '@theia/core/lib/browser/widgets/widget';
2+
import { Widget } from '@theia/core/shared/@phosphor/widgets';
63
import {
74
inject,
85
injectable,
96
postConstruct,
107
} from '@theia/core/shared/inversify';
118
import { DebugWidget as TheiaDebugWidget } from '@theia/debug/lib/browser/view/debug-widget';
129
import { DebugDisabledStatusMessageSource } from '../../contributions/debug';
10+
import {
11+
removeWidgetIfPresent,
12+
unshiftWidgetIfNotPresent,
13+
} from '../dialogs/widgets';
1314

1415
@injectable()
1516
export class DebugWidget extends TheiaDebugWidget {
@@ -38,12 +39,10 @@ export class DebugWidget extends TheiaDebugWidget {
3839
this.messageNode.textContent = message ?? '';
3940
const enabled = !message;
4041
updateVisibility(enabled, this.toolbar, this.sessionWidget);
41-
if (this.layout instanceof PanelLayout) {
42-
if (enabled) {
43-
this.layout.removeWidget(this.statusMessageWidget);
44-
} else {
45-
this.layout.insertWidget(0, this.statusMessageWidget);
46-
}
42+
if (enabled) {
43+
removeWidgetIfPresent(this.layout, this.statusMessageWidget);
44+
} else {
45+
unshiftWidgetIfNotPresent(this.layout, this.statusMessageWidget);
4746
}
4847
this.title.iconClass = enabled ? codicon('debug-alt') : 'fa fa-ban'; // TODO: find a better icon?
4948
});
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import {
2+
Layout,
3+
PanelLayout,
4+
Widget,
5+
} from '@theia/core/shared/@phosphor/widgets';
6+
7+
/**
8+
*
9+
* Removes the widget from the layout if the `layout` is a `PanelLayout` and the widget is present in the layout.
10+
* Otherwise, it's NOOP
11+
* @param layout the layout to remove the widget from. Must be a `PanelLayout`.
12+
* @param toRemove the widget to remove from the layout
13+
*/
14+
export function removeWidgetIfPresent(
15+
layout: Layout | null,
16+
toRemove: Widget
17+
): void {
18+
if (layout instanceof PanelLayout) {
19+
const index = layout.widgets.indexOf(toRemove);
20+
if (index < 0) {
21+
// Unlike the default `PanelLayout#removeWidget` behavior, (https://github.com/phosphorjs/phosphor/blob/9f5e11025b62d2c4a6fb59e2681ae1ed323dcde4/packages/widgets/src/panellayout.ts#L154-L156)
22+
// do not try to remove widget if it's not present (the index is negative).
23+
// Otherwise, required widgets could be removed based on the default ArrayExt behavior (https://github.com/phosphorjs/phosphor/blob/9f5e11025b62d2c4a6fb59e2681ae1ed323dcde4/packages/algorithm/src/array.ts#L1075-L1077)
24+
// See https://github.com/arduino/arduino-ide/issues/2354 for more details.
25+
return;
26+
}
27+
layout.removeWidget(toRemove);
28+
}
29+
}
30+
31+
/**
32+
*
33+
* Inserts the widget to the `0` index of the layout if the `layout` is a `PanelLayout` and the widget is not yet part of the layout.
34+
* Otherwise, it's NOOP
35+
* @param layout the layout to add the widget to. Must be a `PanelLayout`.
36+
* @param toAdd the widget to add to the layout
37+
*/
38+
export function unshiftWidgetIfNotPresent(
39+
layout: Layout | null,
40+
toAdd: Widget
41+
): void {
42+
if (layout instanceof PanelLayout) {
43+
const index = layout.widgets.indexOf(toAdd);
44+
if (index >= 0) {
45+
// Do not try to add the widget to the layout if it's already present.
46+
// This is the counterpart logic of the `removeWidgetIfPresent` function.
47+
return;
48+
}
49+
layout.insertWidget(0, toAdd);
50+
}
51+
}
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import { enableJSDOM } from '@theia/core/lib/browser/test/jsdom';
2+
const disableJSDOM = enableJSDOM();
3+
4+
import { PanelLayout, Widget } from '@theia/core/shared/@phosphor/widgets';
5+
import { expect } from 'chai';
6+
import {
7+
removeWidgetIfPresent,
8+
unshiftWidgetIfNotPresent,
9+
} from '../../browser/theia/dialogs/widgets';
10+
11+
describe('widgets', () => {
12+
after(() => disableJSDOM());
13+
14+
describe('removeWidgetIfPresent', () => {
15+
it('should remove the widget if present', () => {
16+
const layout = new PanelLayout();
17+
const widget = new Widget();
18+
layout.addWidget(widget);
19+
const toRemoveWidget = new Widget();
20+
layout.addWidget(toRemoveWidget);
21+
22+
expect(layout.widgets).to.be.deep.equal([widget, toRemoveWidget]);
23+
24+
removeWidgetIfPresent(layout, toRemoveWidget);
25+
26+
expect(layout.widgets).to.be.deep.equal([widget]);
27+
});
28+
29+
it('should be noop if the widget is not part of the layout', () => {
30+
const layout = new PanelLayout();
31+
const widget = new Widget();
32+
layout.addWidget(widget);
33+
34+
expect(layout.widgets).to.be.deep.equal([widget]);
35+
36+
removeWidgetIfPresent(layout, new Widget());
37+
38+
expect(layout.widgets).to.be.deep.equal([widget]);
39+
});
40+
});
41+
42+
describe('unshiftWidgetIfNotPresent', () => {
43+
it('should unshift the widget if not present', () => {
44+
const layout = new PanelLayout();
45+
const widget = new Widget();
46+
layout.addWidget(widget);
47+
48+
expect(layout.widgets).to.be.deep.equal([widget]);
49+
50+
const toAdd = new Widget();
51+
unshiftWidgetIfNotPresent(layout, toAdd);
52+
53+
expect(layout.widgets).to.be.deep.equal([toAdd, widget]);
54+
});
55+
56+
it('should be NOOP if widget is already part of the layout (at 0 index)', () => {
57+
const layout = new PanelLayout();
58+
const toAdd = new Widget();
59+
layout.addWidget(toAdd);
60+
const widget = new Widget();
61+
layout.addWidget(widget);
62+
63+
expect(layout.widgets).to.be.deep.equal([toAdd, widget]);
64+
65+
unshiftWidgetIfNotPresent(layout, toAdd);
66+
67+
expect(layout.widgets).to.be.deep.equal([toAdd, widget]);
68+
});
69+
70+
it('should be NOOP if widget is already part of the layout (at >0 index)', () => {
71+
const layout = new PanelLayout();
72+
const widget = new Widget();
73+
layout.addWidget(widget);
74+
const toAdd = new Widget();
75+
layout.addWidget(toAdd);
76+
77+
expect(layout.widgets).to.be.deep.equal([widget, toAdd]);
78+
79+
unshiftWidgetIfNotPresent(layout, toAdd);
80+
81+
expect(layout.widgets).to.be.deep.equal([widget, toAdd]);
82+
});
83+
});
84+
});

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /