fix: debug widget layout updates

Use customized `PanelLayout#removeWidget` and `PanelLayout#insertWidget`
functions. The customized functions ensure no side effect when
adding/removing the widget to/from the layout but it's already
present/absent.

Closes: #2354
Signed-off-by: Akos Kitta <[email protected]>

Author: Akos Kitta

Diff for: arduino-ide-extension/src/browser/theia/debug/debug-widget.ts

@@ -1,15 +1,16 @@
-import {
-  codicon,
-  PanelLayout,
-  Widget,
-} from '@theia/core/lib/browser/widgets/widget';
+import { codicon } from '@theia/core/lib/browser/widgets/widget';
+import { Widget } from '@theia/core/shared/@phosphor/widgets';
 import {
   inject,
   injectable,
   postConstruct,
 } from '@theia/core/shared/inversify';
 import { DebugWidget as TheiaDebugWidget } from '@theia/debug/lib/browser/view/debug-widget';
 import { DebugDisabledStatusMessageSource } from '../../contributions/debug';
+import {
+  removeWidgetIfPresent,
+  unshiftWidgetIfNotPresent,
+} from '../dialogs/widgets';
 
 @injectable()
 export class DebugWidget extends TheiaDebugWidget {
@@ -38,12 +39,10 @@ export class DebugWidget extends TheiaDebugWidget {
       this.messageNode.textContent = message ?? '';
       const enabled = !message;
       updateVisibility(enabled, this.toolbar, this.sessionWidget);
-      if (this.layout instanceof PanelLayout) {
-        if (enabled) {
-          this.layout.removeWidget(this.statusMessageWidget);
-        } else {
-          this.layout.insertWidget(0, this.statusMessageWidget);
-        }
+      if (enabled) {
+        removeWidgetIfPresent(this.layout, this.statusMessageWidget);
+      } else {
+        unshiftWidgetIfNotPresent(this.layout, this.statusMessageWidget);
       }
       this.title.iconClass = enabled ? codicon('debug-alt') : 'fa fa-ban'; // TODO: find a better icon?
     });

Diff for: arduino-ide-extension/src/browser/theia/dialogs/widgets.ts

@@ -0,0 +1,51 @@
+import {
+  Layout,
+  PanelLayout,
+  Widget,
+} from '@theia/core/shared/@phosphor/widgets';
+
+/**
+ *
+ * Removes the widget from the layout if the `layout` is a `PanelLayout` and the widget is present in the layout.
+ * Otherwise, it's NOOP
+ * @param layout the layout to remove the widget from. Must be a `PanelLayout`.
+ * @param toRemove the widget to remove from the layout
+ */
+export function removeWidgetIfPresent(
+  layout: Layout | null,
+  toRemove: Widget
+): void {
+  if (layout instanceof PanelLayout) {
+    const index = layout.widgets.indexOf(toRemove);
+    if (index < 0) {
+      // Unlike the default `PanelLayout#removeWidget` behavior, (https://github.com/phosphorjs/phosphor/blob/9f5e11025b62d2c4a6fb59e2681ae1ed323dcde4/packages/widgets/src/panellayout.ts#L154-L156)
+      // do not try to remove widget if it's not present (the index is negative).
+      // 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)
+      // See https://github.com/arduino/arduino-ide/issues/2354 for more details.
+      return;
+    }
+    layout.removeWidget(toRemove);
+  }
+}
+
+/**
+ *
+ * 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.
+ * Otherwise, it's NOOP
+ * @param layout the layout to add the widget to. Must be a `PanelLayout`.
+ * @param toAdd the widget to add to the layout
+ */
+export function unshiftWidgetIfNotPresent(
+  layout: Layout | null,
+  toAdd: Widget
+): void {
+  if (layout instanceof PanelLayout) {
+    const index = layout.widgets.indexOf(toAdd);
+    if (index >= 0) {
+      // Do not try to add the widget to the layout if it's already present.
+      // This is the counterpart logic of the `removeWidgetIfPresent` function.
+      return;
+    }
+    layout.insertWidget(0, toAdd);
+  }
+}

Diff for: arduino-ide-extension/src/test/browser/widgets.test.ts

@@ -0,0 +1,84 @@
+import { enableJSDOM } from '@theia/core/lib/browser/test/jsdom';
+const disableJSDOM = enableJSDOM();
+
+import { PanelLayout, Widget } from '@theia/core/shared/@phosphor/widgets';
+import { expect } from 'chai';
+import {
+  removeWidgetIfPresent,
+  unshiftWidgetIfNotPresent,
+} from '../../browser/theia/dialogs/widgets';
+
+describe('widgets', () => {
+  after(() => disableJSDOM());
+
+  describe('removeWidgetIfPresent', () => {
+    it('should remove the widget if present', () => {
+      const layout = new PanelLayout();
+      const widget = new Widget();
+      layout.addWidget(widget);
+      const toRemoveWidget = new Widget();
+      layout.addWidget(toRemoveWidget);
+
+      expect(layout.widgets).to.be.deep.equal([widget, toRemoveWidget]);
+
+      removeWidgetIfPresent(layout, toRemoveWidget);
+
+      expect(layout.widgets).to.be.deep.equal([widget]);
+    });
+
+    it('should be noop if the widget is not part of the layout', () => {
+      const layout = new PanelLayout();
+      const widget = new Widget();
+      layout.addWidget(widget);
+
+      expect(layout.widgets).to.be.deep.equal([widget]);
+
+      removeWidgetIfPresent(layout, new Widget());
+
+      expect(layout.widgets).to.be.deep.equal([widget]);
+    });
+  });
+
+  describe('unshiftWidgetIfNotPresent', () => {
+    it('should unshift the widget if not present', () => {
+      const layout = new PanelLayout();
+      const widget = new Widget();
+      layout.addWidget(widget);
+
+      expect(layout.widgets).to.be.deep.equal([widget]);
+
+      const toAdd = new Widget();
+      unshiftWidgetIfNotPresent(layout, toAdd);
+
+      expect(layout.widgets).to.be.deep.equal([toAdd, widget]);
+    });
+
+    it('should be NOOP if widget is already part of the layout (at 0 index)', () => {
+      const layout = new PanelLayout();
+      const toAdd = new Widget();
+      layout.addWidget(toAdd);
+      const widget = new Widget();
+      layout.addWidget(widget);
+
+      expect(layout.widgets).to.be.deep.equal([toAdd, widget]);
+
+      unshiftWidgetIfNotPresent(layout, toAdd);
+
+      expect(layout.widgets).to.be.deep.equal([toAdd, widget]);
+    });
+
+    it('should be NOOP if widget is already part of the layout (at >0 index)', () => {
+      const layout = new PanelLayout();
+      const widget = new Widget();
+      layout.addWidget(widget);
+      const toAdd = new Widget();
+      layout.addWidget(toAdd);
+
+      expect(layout.widgets).to.be.deep.equal([widget, toAdd]);
+
+      unshiftWidgetIfNotPresent(layout, toAdd);
+
+      expect(layout.widgets).to.be.deep.equal([widget, toAdd]);
+    });
+  });
+});