fix: check for updates dialog

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
arduino · kittaakos · Nov 29, 2022 · Jul 22, 2022 · Nov 22, 2022 · Nov 22, 2022
commit 3b8ed7f4dfaf91ea8808bc8be493c0494cb9534a
diff --git a/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts b/arduino-ide-extension/src/browser/arduino-ide-frontend-module.ts
@@ -262,7 +262,6 @@ import { IDEUpdaterClientImpl } from './ide-updater/ide-updater-client-impl';
 import {
   IDEUpdaterDialog,
   IDEUpdaterDialogProps,
-  IDEUpdaterDialogWidget,
 } from './dialogs/ide-updater/ide-updater-dialog';
 import { ElectronIpcConnectionProvider } from '@theia/core/lib/electron-browser/messaging/electron-ipc-connection-provider';
 import { MonitorModel } from './monitor-model';
@@ -910,7 +909,6 @@ export default new ContainerModule((bind, unbind, isBound, rebind) => {
     title: 'UploadCertificate',
   });
 
-  bind(IDEUpdaterDialogWidget).toSelf().inSingletonScope();
   bind(IDEUpdaterDialog).toSelf().inSingletonScope();
   bind(IDEUpdaterDialogProps).toConstantValue({
     title: 'IDEUpdater',

diff --git a/arduino-ide-extension/src/browser/dialogs/ide-updater/ide-updater-component.tsx b/arduino-ide-extension/src/browser/dialogs/ide-updater/ide-updater-component.tsx
@@ -1,7 +1,6 @@
 import { nls } from '@theia/core/lib/common';
-import { shell } from 'electron';
+import { shell } from '@theia/core/electron-shared/@electron/remote';
 import * as React from '@theia/core/shared/react';
-import { createRoot } from '@theia/core/shared/react-dom/client';
 import ReactMarkdown from 'react-markdown';
 import { ProgressInfo, UpdateInfo } from '../../../common/protocol/ide-updater';
 import ProgressBar from '../../components/ProgressBar';
@@ -28,32 +27,19 @@ export const IDEUpdaterComponent = ({
   },
 }: IDEUpdaterComponentProps): React.ReactElement => {
   const { version, releaseNotes } = updateInfo;
-  const changelogDivRef =
-    React.useRef() as React.MutableRefObject<HTMLDivElement>;
-  const changelogRoot = createRoot(changelogDivRef.current);
+  const [changelog, setChangelog] = React.useState<string>('');
   React.useEffect(() => {
-    if (!!releaseNotes && changelogDivRef.current) {
-      let changelog: string;
-      if (typeof releaseNotes === 'string') changelog = releaseNotes;
-      else
-        changelog = releaseNotes.reduce((acc, item) => {
-          return item.note ? (acc += `${item.note}\n\n`) : acc;
-        }, '');
-      changelogRoot.render(
-        <ReactMarkdown
-          components={{
-            a: ({ href, children, ...props }) => (
-              <a onClick={() => href && shell.openExternal(href)} {...props}>
-                {children}
-              </a>
-            ),
-          }}
-        >
-          {changelog}
-        </ReactMarkdown>
+    if (releaseNotes) {
+      setChangelog(
+        typeof releaseNotes === 'string'
+          ? releaseNotes
+          : releaseNotes.reduce(
+              (acc, item) => (item.note ? (acc += `${item.note}\n\n`) : acc),
+              ''
+            )
       );
     }
-  }, [updateInfo]);
+  }, [releaseNotes, changelog]);
 
   const DownloadCompleted: () => React.ReactElement = () => (
     <div className="ide-updater-dialog--downloaded">
@@ -106,9 +92,24 @@ export const IDEUpdaterComponent = ({
             version
           )}
         </div>
-        {releaseNotes && (
+        {changelog && (
           <div className="dialogRow changelog-container">
-            <div className="changelog" ref={changelogDivRef} />
+            <div className="changelog">
+              <ReactMarkdown
+                components={{
+                  a: ({ href, children, ...props }) => (
+                    <a
+                      onClick={() => href && shell.openExternal(href)}
+                      {...props}
+                    >
+                      {children}
+                    </a>
+                  ),
+                }}
+              >
+                {changelog}
+              </ReactMarkdown>
+            </div>
           </div>
         )}
       </div>

diff --git a/arduino-ide-extension/src/browser/dialogs/ide-updater/ide-updater-dialog.tsx b/arduino-ide-extension/src/browser/dialogs/ide-updater/ide-updater-dialog.tsx
@@ -5,10 +5,8 @@ import {
   postConstruct,
 } from '@theia/core/shared/inversify';
 import { DialogProps } from '@theia/core/lib/browser/dialogs';
-import { AbstractDialog } from '../../theia/dialogs/dialogs';
-import { Widget } from '@theia/core/shared/@phosphor/widgets';
 import { Message } from '@theia/core/shared/@phosphor/messaging';
-import { ReactWidget } from '@theia/core/lib/browser/widgets/react-widget';
+import { ReactDialog } from '../../theia/dialogs/dialogs';
 import { nls } from '@theia/core';
 import { IDEUpdaterComponent, UpdateProgress } from './ide-updater-component';
 import {
@@ -22,47 +20,11 @@ import { WindowService } from '@theia/core/lib/browser/window/window-service';
 
 const DOWNLOAD_PAGE_URL = 'https://www.arduino.cc/en/software';
 
-@injectable()
-export class IDEUpdaterDialogWidget extends ReactWidget {
-  private _updateInfo: UpdateInfo;
-  private _updateProgress: UpdateProgress = {};
-
-  setUpdateInfo(updateInfo: UpdateInfo): void {
-    this._updateInfo = updateInfo;
-    this.update();
-  }
-
-  mergeUpdateProgress(updateProgress: UpdateProgress): void {
-    this._updateProgress = { ...this._updateProgress, ...updateProgress };
-    this.update();
-  }
-
-  get updateInfo(): UpdateInfo {
-    return this._updateInfo;
-  }
-
-  get updateProgress(): UpdateProgress {
-    return this._updateProgress;
-  }
-
-  protected render(): React.ReactNode {
-    return !!this._updateInfo ? (
-      <IDEUpdaterComponent
-        updateInfo={this._updateInfo}
-        updateProgress={this._updateProgress}
-      />
-    ) : null;
-  }
-}
-
 @injectable()
 export class IDEUpdaterDialogProps extends DialogProps {}
 
 @injectable()
-export class IDEUpdaterDialog extends AbstractDialog<UpdateInfo> {
-  @inject(IDEUpdaterDialogWidget)
-  private readonly widget: IDEUpdaterDialogWidget;
-
+export class IDEUpdaterDialog extends ReactDialog<UpdateInfo | undefined> {
   @inject(IDEUpdater)
   private readonly updater: IDEUpdater;
 
@@ -75,6 +37,9 @@ export class IDEUpdaterDialog extends AbstractDialog<UpdateInfo> {
   @inject(WindowService)
   private readonly windowService: WindowService;
 
+  private _updateInfo: UpdateInfo | undefined;
+  private _updateProgress: UpdateProgress = {};
+
   constructor(
     @inject(IDEUpdaterDialogProps)
     protected override readonly props: IDEUpdaterDialogProps
@@ -94,26 +59,34 @@ export class IDEUpdaterDialog extends AbstractDialog<UpdateInfo> {
   protected init(): void {
     this.updaterClient.onUpdaterDidFail((error) => {
       this.appendErrorButtons();
-      this.widget.mergeUpdateProgress({ error });
+      this.mergeUpdateProgress({ error });
     });
     this.updaterClient.onDownloadProgressDidChange((progressInfo) => {
-      this.widget.mergeUpdateProgress({ progressInfo });
+      this.mergeUpdateProgress({ progressInfo });
     });
     this.updaterClient.onDownloadDidFinish(() => {
       this.appendInstallButtons();
-      this.widget.mergeUpdateProgress({ downloadFinished: true });
+      this.mergeUpdateProgress({ downloadFinished: true });
     });
   }
 
-  get value(): UpdateInfo {
-    return this.widget.updateInfo;
+  protected render(): React.ReactNode {
+    return (
+      this.updateInfo && (
+        <IDEUpdaterComponent
+          updateInfo={this.updateInfo}
+          updateProgress={this.updateProgress}
+        /&
6DB6
gt;
+      )
+    );
+  }
+
+  get value(): UpdateInfo | undefined {
+    return this.updateInfo;
   }
 
   protected override onAfterAttach(msg: Message): void {
-    if (this.widget.isAttached) {
-      Widget.detach(this.widget);
-    }
-    Widget.attach(this.widget, this.contentNode);
+    this.update();
     this.appendInitialButtons();
     super.onAfterAttach(msg);
   }
@@ -196,15 +169,19 @@ export class IDEUpdaterDialog extends AbstractDialog<UpdateInfo> {
   }
 
   private skipVersion(): void {
+    if (!this.updateInfo) {
+      console.warn(`Nothing to skip. No update info is available`);
+      return;
+    }
     this.localStorageService.setData<string>(
       SKIP_IDE_VERSION,
-      this.widget.updateInfo.version
+      this.updateInfo.version
     );
     this.close();
   }
 
   private startDownload(): void {
-    this.widget.mergeUpdateProgress({
+    this.mergeUpdateProgress({
       downloadStarted: true,
     });
     this.clearButtons();
@@ -216,31 +193,48 @@ export class IDEUpdaterDialog extends AbstractDialog<UpdateInfo> {
     this.close();
   }
 
+  private set updateInfo(updateInfo: UpdateInfo | undefined) {
+    this._updateInfo = updateInfo;
+    this.update();
+  }
+
+  private get updateInfo(): UpdateInfo | undefined {
+    return this._updateInfo;
+  }
+
+  private get updateProgress(): UpdateProgress {
+    return this._updateProgress;
+  }
+
+  private mergeUpdateProgress(updateProgress: UpdateProgress): void {
+    this._updateProgress = { ...this._updateProgress, ...updateProgress };
+    this.update();
+  }
+
   override async open(
     data: UpdateInfo | undefined = undefined
   ): Promise<UpdateInfo | undefined> {
     if (data && data.version) {
-      this.widget.mergeUpdateProgress({
+      this.mergeUpdateProgress({
         progressInfo: undefined,
         downloadStarted: false,
         downloadFinished: false,
         error: undefined,
       });
-      this.widget.setUpdateInfo(data);
+      this.updateInfo = data;
       return super.open();
     }
   }
 
   protected override onActivateRequest(msg: Message): void {
     super.onActivateRequest(msg);
-    this.widget.activate();
+    this.update();
   }
 
   override close(): void {
-    this.widget.dispose();
     if (
-      this.widget.updateProgress?.downloadStarted &&
-      !this.widget.updateProgress?.downloadFinished
+      this.updateProgress?.downloadStarted &&
+      !this.updateProgress?.downloadFinished
     ) {
       this.updater.stopDownload();
     }

diff --git a/arduino-ide-extension/src/browser/theia/dialogs/dialogs.ts b/arduino-ide-extension/src/browser/theia/dialogs/dialogs.ts
diff --git a/arduino-ide-extension/src/browser/theia/dialogs/dialogs.tsx b/arduino-ide-extension/src/browser/theia/dialogs/dialogs.tsx
@@ -0,0 +1,63 @@
+import {
+  AbstractDialog as TheiaAbstractDialog,
+  DialogProps,
+} from '@theia/core/lib/browser/dialogs';
+import { ReactDialog as TheiaReactDialog } from '@theia/core/lib/browser/dialogs/react-dialog';
+import { codiconArray, Message } from '@theia/core/lib/browser/widgets/widget';
+import {
+  Disposable,
+  DisposableCollection,
+} from '@theia/core/lib/common/disposable';
+import { inject, injectable } from '@theia/core/shared/inversify';
+import * as React from '@theia/core/shared/react';
+import { createRoot } from '@theia/core/shared/react-dom/client';
+
+@injectable()
+export abstract class AbstractDialog<T> extends TheiaAbstractDialog<T> {
+  constructor(
+    @inject(DialogProps) protected override readonly props: DialogProps
+  ) {
+    super(props);
+
+    this.closeCrossNode.classList.remove(...codiconArray('close'));
+    this.closeCrossNode.classList.add('fa', 'fa-close');
+  }
+}
+
+@injectable()
+export abstract class ReactDialog<T> extends TheiaReactDialog<T> {
+  protected override onUpdateRequest(msg: Message): void {
+    // This is tricky to bypass the default Theia code.
+    // Otherwise, there is a warning when opening the dialog for the second time.
+    // You are calling ReactDOMClient.createRoot() on a container that has already been passed to createRoot() before. Instead, call root.render() on the existing root instead if you want to update it.
+    const disposables = new DisposableCollection();
+    if (!this.isMounted) {
+      // toggle the `isMounted` logic for the time being of the super call so that the `createRoot` does not run
+      this.isMounted = true;
+      disposables.push(Disposable.create(() => (this.isMounted = false)));
+    }
+
+    // Always unset the `contentNodeRoot` so there is no double update when calling super.
+    const restoreContentNodeRoot = this.contentNodeRoot;
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    (this.contentNodeRoot as any) = undefined;
+    disposables.push(
+      Disposable.create(() => (this.contentNodeRoot = restoreContentNodeRoot))
+    );
+
+    try {
+      super.onUpdateRequest(msg);
+    } finally {
+      disposables.dispose();
+    }
+
+    // Use the patched rendering.
+    if (!this.isMounted) {
+      this.contentNodeRoot = createRoot(this.contentNode);
+      // Resetting the prop is missing from the Theia code.
+      // https://github.com/eclipse-theia/theia/blob/v1.31.1/packages/core/src/browser/dialogs/react-dialog.tsx#L41-L47
+      this.isMounted = true;
+    }
+    this.contentNodeRoot?.render(<>{this.render()}</>);
+  }
+}