Make root frame child components comply with open closed principle and include it in the behavioural unit tests

Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>

src/behaviours/add-cluster/navigation-using-application-menu.test.tsx

@@ -9,10 +9,6 @@ import { getApplicationBuilder } from "../../renderer/components/test-utils/get-
 import React from "react";
 
 // TODO: Make components free of side effects by making them deterministic
-jest.mock("../../renderer/components/tooltip/tooltip", () => ({
-  Tooltip: () => null,
-}));
-
 jest.mock("../../renderer/components/tooltip/withTooltip", () => ({
   withTooltip: (Target: any) => ({ tooltip, tooltipOverrideDisabled, ...props }: any) => <Target {...props} />,
 }));

src/behaviours/welcome/navigation-using-application-menu.test.ts

@@ -21,25 +21,41 @@ describe("welcome - navigation using application menu", () => {
     expect(rendered.container).toMatchSnapshot();
   });
 
-  it("does not show welcome page yet", () => {
-    const actual = rendered.queryByTestId("welcome-page");
+  it("shows welcome page being front page", () => {
+    const actual = rendered.getByTestId("welcome-page");
 
-    expect(actual).toBeNull();
+    expect(actual).not.toBeNull();
   });
 
-  describe("when navigating to welcome using application menu", () => {
+  describe("when navigated somewhere else", () => {
     beforeEach(() => {
-      applicationBuilder.applicationMenu.click("help.welcome");
+      applicationBuilder.applicationMenu.click("root.preferences");
     });
 
     it("renders", () => {
-      expect(rendered.container).toMatchSnapshot();
+      expect(rendered.baseElement).toMatchSnapshot();
     });
 
-    it("shows welcome page", () => {
-      const actual = rendered.getByTestId("welcome-page");
+    it("does not show welcome page", () => {
+      const actual = rendered.queryByTestId("welcome-page");
 
-      expect(actual).not.toBeNull();
+      expect(actual).toBeNull();
+    });
+
+    describe("when navigated to welcome using application menu", () => {
+      beforeEach(() => {
+        applicationBuilder.applicationMenu.click("help.welcome");
+      });
+
+      it("renders", () => {
+        expect(rendered.container).toMatchSnapshot();
+      });
+
+      it("shows welcome page", () => {
+        const actual = rendered.getByTestId("welcome-page");
+
+        expect(actual).not.toBeNull();
+      });
     });
   });
 });

src/main/getDiForUnitTesting.ts

@@ -99,6 +99,7 @@ import updateHelmReleaseInjectable from "./helm/helm-service/update-helm-release
 import waitUntilBundledExtensionsAreLoadedInjectable from "./start-main-application/lens-window/application-window/wait-until-bundled-extensions-are-loaded.injectable";
 import { registerMobX } from "@ogre-tools/injectable-extension-for-mobx";
 import electronInjectable from "./utils/resolve-system-proxy/electron.injectable";
+import type { HotbarStore } from "../common/hotbars/store";
 
 export function getDiForUnitTesting(opts: { doGeneralOverrides?: boolean } = {}) {
   const {
@@ -127,7 +128,13 @@ export function getDiForUnitTesting(opts: { doGeneralOverrides?: boolean } = {})
     di.override(electronInjectable, () => ({}));
     di.override(waitUntilBundledExtensionsAreLoadedInjectable, () => async () => {});
     di.override(getRandomIdInjectable, () => () => "some-irrelevant-random-id");
-    di.override(hotbarStoreInjectable, () => ({ load: () => {} }));
+
+    di.override(hotbarStoreInjectable, () => ({
+      load: () => {},
+      getActive: () => ({ name: "some-hotbar", items: [] }),
+      getDisplayIndex: () => "0",
+    }) as unknown as HotbarStore);
+
     di.override(userStoreInjectable, () => ({ startMainReactions: () => {}, extensionRegistryUrl: { customUrl: "some-custom-url" }}) as UserStore);
     di.override(extensionsStoreInjectable, () => ({ isEnabled: (opts) => (void opts, false) }) as ExtensionsStore);
     di.override(clusterStoreInjectable, () => ({ provideInitialFromMain: () => {}, getById: (id) => (void id, {}) as Cluster }) as ClusterStore);

src/renderer/components/cluster-manager/cluster-manager-root-frame-child-component.injectable.tsx

@@ -0,0 +1,30 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import React from "react";
+import { getInjectable } from "@ogre-tools/injectable";
+import { rootFrameChildComponentInjectionToken } from "../../frames/root-frame/root-frame-child-component-injection-token";
+import { ClusterManager } from "./cluster-manager";
+import { computed } from "mobx";
+import { ErrorBoundary } from "../error-boundary";
+
+const clusterManagerRootFrameChildComponentInjectable = getInjectable({
+  id: "cluster-manager-root-frame-child-component",
+
+  instantiate: () => ({
+    id: "cluster-manager",
+
+    shouldRender: computed(() => true),
+
+    Component: () => (
+      <ErrorBoundary>
+        <ClusterManager />
+      </ErrorBoundary>
+    ),
+  }),
+
+  injectionToken: rootFrameChildComponentInjectionToken,
+});
+
+export default clusterManagerRootFrameChildComponentInjectable;

src/renderer/components/command-palette/command-container-root-frame-child-component.injectable.ts

@@ -0,0 +1,24 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import { getInjectable } from "@ogre-tools/injectable";
+import { rootFrameChildComponentInjectionToken } from "../../frames/root-frame/root-frame-child-component-injection-token";
+import { computed } from "mobx";
+import { CommandContainer } from "./command-container";
+
+const commandContainerRootFrameChildComponentInjectable = getInjectable({
+  id: "command-container-root-frame-child-component",
+
+  instantiate: () => ({
+    id: "command-container",
+    shouldRender: computed(() => true),
+    Component: CommandContainer,
+  }),
+
+  causesSideEffects: true,
+
+  injectionToken: rootFrameChildComponentInjectionToken,
+});
+
+export default commandContainerRootFrameChildComponentInjectable;

src/renderer/components/confirm-dialog/confirm-dialog-root-frame-child-component.injectable.ts

@@ -0,0 +1,22 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import { getInjectable } from "@ogre-tools/injectable";
+import { rootFrameChildComponentInjectionToken } from "../../frames/root-frame/root-frame-child-component-injection-token";
+import { computed } from "mobx";
+import { ConfirmDialog } from "./confirm-dialog";
+
+const confirmDialogRootFrameChildComponentInjectable = getInjectable({
+  id: "confirm-dialog-root-frame-child-component",
+
+  instantiate: () => ({
+    id: "confirm-dialog",
+    shouldRender: computed(() => true),
+    Component: ConfirmDialog,
+  }),
+
+  injectionToken: rootFrameChildComponentInjectionToken,
+});
+
+export default confirmDialogRootFrameChildComponentInjectable;

src/renderer/components/delete-cluster-dialog/__tests__/delete-cluster-dialog.test.tsx

@@ -24,7 +24,6 @@ import { getInjectable } from "@ogre-tools/injectable";
 import { computed } from "mobx";
 import { routeSpecificComponentInjectionToken } from "../../../routes/route-specific-component-injection-token";
 import { navigateToRouteInjectionToken } from "../../../../common/front-end-routing/navigate-to-route-injection-token";
-import hotbarStoreInjectable from "../../../../common/hotbars/store.injectable";
 import normalizedPlatformInjectable from "../../../../common/vars/normalized-platform.injectable";
 import kubectlBinaryNameInjectable from "../../../../main/kubectl/binary-name.injectable";
 import kubectlDownloadingNormalizedArchInjectable from "../../../../main/kubectl/normalized-arch.injectable";
@@ -111,8 +110,6 @@ describe("<DeleteClusterDialog />", () => {
       mainDi.override(kubectlBinaryNameInjectable, () => "kubectl");
       mainDi.override(kubectlDownloadingNormalizedArchInjectable, () => "amd64");
       mainDi.override(normalizedPlatformInjectable, () => "darwin");
-
-      rendererDi.override(hotbarStoreInjectable, () => ({}));
       rendererDi.override(storesAndApisCanBeCreatedInjectable, () => true);
     });
 

src/renderer/components/notifications/notifications-root-frame-child-component.injectable.ts

@@ -0,0 +1,22 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import { getInjectable } from "@ogre-tools/injectable";
+import { rootFrameChildComponentInjectionToken } from "../../frames/root-frame/root-frame-child-component-injection-token";
+import { computed } from "mobx";
+import { Notifications } from "./notifications";
+
+const notificationsRootFrameChildComponentInjectable = getInjectable({
+  id: "notifications-root-frame-child-component",
+
+  instantiate: () => ({
+    id: "notifications",
+    shouldRender: computed(() => true),
+    Component: Notifications,
+  }),
+
+  injectionToken: rootFrameChildComponentInjectionToken,
+});
+
+export default notificationsRootFrameChildComponentInjectable;

src/renderer/components/test-utils/get-application-builder.tsx

@@ -7,7 +7,6 @@ import rendererExtensionsInjectable from "../../../extensions/renderer-extension
 import currentlyInClusterFrameInjectable from "../../routes/currently-in-cluster-frame.injectable";
 import type { IObservableArray, ObservableSet } from "mobx";
 import { computed, observable, runInAction } from "mobx";
-import { renderFor } from "./renderFor";
 import React from "react";
 import { Router } from "react-router";
 import { Observer } from "mobx-react";
@@ -45,7 +44,6 @@ import type { MinimalTrayMenuItem } from "../../../main/tray/electron-tray/elect
 import electronTrayInjectable from "../../../main/tray/electron-tray/electron-tray.injectable";
 import applicationWindowInjectable from "../../../main/start-main-application/lens-window/application-window/application-window.injectable";
 import { Notifications } from "../notifications/notifications";
-import broadcastThatRootFrameIsRenderedInjectable from "../../frames/root-frame/broadcast-that-root-frame-is-rendered.injectable";
 import { getDiForUnitTesting as getRendererDi } from "../../getDiForUnitTesting";
 import { getDiForUnitTesting as getMainDi } from "../../../main/getDiForUnitTesting";
 import { overrideChannels } from "../../../test-utils/channel-fakes/override-channels";
@@ -53,7 +51,6 @@ import trayIconPathsInjectable from "../../../main/tray/tray-icon-path.injectabl
 import assert from "assert";
 import { openMenu } from "react-select-event";
 import userEvent from "@testing-library/user-event";
-import { StatusBar } from "../status-bar/status-bar";
 import lensProxyPortInjectable from "../../../main/lens-proxy/lens-proxy-port.injectable";
 import type { Route } from "../../../common/front-end-routing/front-end-route-injection-token";
 import type { NavigateToRouteOptions } from "../../../common/front-end-routing/navigate-to-route-injection-token";
@@ -62,7 +59,8 @@ import type { LensMainExtension } from "../../../extensions/lens-main-extension"
 import type { LensExtension } from "../../../extensions/lens-extension";
 
 import extensionInjectable from "../../../extensions/extension-loader/extension/extension.injectable";
-import { TopBar } from "../layout/top-bar/top-bar";
+import { renderFor } from "./renderFor";
+import { RootFrame } from "../../frames/root-frame/root-frame";
 
 type Callback = (dis: DiContainers) => void | Promise<void>;
 
@@ -125,10 +123,7 @@ interface DiContainers {
 }
 
 interface Environment {
-  renderSidebar: () => React.ReactNode;
-  renderTopBar: () => React.ReactNode;
-  renderStatusBar: () => React.ReactNode;
-  beforeRender: () => void;
+  render: () => RenderResult;
   onAllowKubeResource: () => void;
 }
 
@@ -166,16 +161,16 @@ export const getApplicationBuilder = () => {
 
   const environments = {
     application: {
-      renderSidebar: () => null,
+      render: () => {
+        const history = rendererDi.inject(historyInjectable);
 
-      renderTopBar: () => <TopBar />,
+        const render = renderFor(rendererDi);
 
-      renderStatusBar: () => <StatusBar />,
-
-      beforeRender: () => {
-        const nofifyThatRootFrameIsRendered = rendererDi.inject(broadcastThatRootFrameIsRenderedInjectable);
-
-        nofifyThatRootFrameIsRendered();
+        return render(
+          <Router history={history}>
+            <RootFrame />
+          </Router>,
+        );
       },
 
       onAllowKubeResource: () => {
@@ -186,10 +181,33 @@ export const getApplicationBuilder = () => {
     } as Environment,
 
     clusterFrame: {
-      renderSidebar: () => <Sidebar />,
-      renderStatusBar: () => null,
-      renderTopBar: () => null,
-      beforeRender: () => {},
+      render: () => {
+        const currentRouteComponent = rendererDi.inject(currentRouteComponentInjectable);
+        const history = rendererDi.inject(historyInjectable);
+
+        const render = renderFor(rendererDi);
+
+        return render(
+          <Router history={history}>
+            <Sidebar />
+
+            <Observer>
+              {() => {
+                const Component = currentRouteComponent.get();
+
+                if (!Component) {
+                  return null;
+                }
+
+                return <Component />;
+              }}
+            </Observer>
+
+            <Notifications />
+          </Router>,
+        );
+      },
+
       onAllowKubeResource: () => {},
     } as Environment,
   };
@@ -474,37 +492,12 @@ export const getApplicationBuilder = () => {
 
       await startFrame();
 
-      const render = renderFor(rendererDi);
-      const history = rendererDi.inject(historyInjectable);
-      const currentRouteComponent = rendererDi.inject(currentRouteComponentInjectable);
 
       for (const callback of beforeRenderCallbacks) {
         await callback(dis);
       }
 
-      environment.beforeRender();
-
-      rendered = render(
-        <Router history={history}>
-          {environment.renderSidebar()}
-          {environment.renderTopBar()}
-          {environment.renderStatusBar()}
-
-          <Observer>
-            {() => {
-              const Component = currentRouteComponent.get();
-
-              if (!Component) {
-                return null;
-              }
-
-              return <Component />;
-            }}
-          </Observer>
-
-          <Notifications />
-        </Router>,
-      );
+      rendered = environment.render();
 
       return rendered;
     },

src/renderer/frames/root-frame/root-frame-child-component-injection-token.ts

@@ -0,0 +1,17 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+
+import { getInjectionToken } from "@ogre-tools/injectable";
+import type { IComputedValue } from "mobx";
+
+export interface RootFrameChildComponent {
+  id: string;
+  Component: React.ElementType;
+  shouldRender: IComputedValue<boolean>;
+}
+
+export const rootFrameChildComponentInjectionToken = getInjectionToken<RootFrameChildComponent>({
+  id: "root-frame-child-component",
+});

src/renderer/frames/root-frame/root-frame.tsx

@@ -5,23 +5,20 @@
 
 import { injectSystemCAs } from "../../../common/system-ca";
 import React from "react";
-import { observer } from "mobx-react";
-import { ClusterManager } from "../../components/cluster-manager";
-import { ErrorBoundary } from "../../components/error-boundary";
-import { Notifications } from "../../components/notifications";
-import { ConfirmDialog } from "../../components/confirm-dialog";
-import { CommandContainer } from "../../components/command-palette/command-container";
+import { Observer } from "mobx-react";
 import { withInjectables } from "@ogre-tools/injectable-react";
 import broadcastThatRootFrameIsRenderedInjectable from "./broadcast-that-root-frame-is-rendered.injectable";
+import type { RootFrameChildComponent } from "./root-frame-child-component-injection-token";
+import { rootFrameChildComponentInjectionToken } from "./root-frame-child-component-injection-token";
 
 // Todo: remove import-time side-effect.
 injectSystemCAs();
 
 interface Dependencies {
   broadcastThatRootFrameIsRendered: () => void;
+  childComponents: RootFrameChildComponent[];
 }
 
-@observer
 class NonInjectedRootFrame extends React.Component<Dependencies> {
   static displayName = "RootFrame";
 
@@ -32,12 +29,12 @@ class NonInjectedRootFrame extends React.Component<Dependencies> {
   render() {
     return (
       <>
-        <ErrorBoundary>
-          <ClusterManager />
-        </ErrorBoundary>
-        <Notifications />
-        <ConfirmDialog />
-        <CommandContainer />
+        {this.props.childComponents
+          .map((child) => (
+            <Observer key={child.id}>
+              {() => (child.shouldRender.get() ? <child.Component /> : null) }
+            </Observer>
+          ))}
       </>
     );
   }
@@ -49,6 +46,7 @@ export const RootFrame = withInjectables<Dependencies>(
   {
     getProps: (di, props) => ({
       broadcastThatRootFrameIsRendered: di.inject(broadcastThatRootFrameIsRenderedInjectable),
+      childComponents: di.injectMany(rootFrameChildComponentInjectionToken),
       ...props,
     }),
   },

src/renderer/getDiForUnitTesting.tsx

@@ -41,7 +41,7 @@ import apiManagerInjectable from "../common/k8s-api/api-manager/manager.injectab
 import ipcRendererInjectable from "./utils/channel/ipc-renderer.injectable";
 import type { IpcRenderer } from "electron";
 import setupOnApiErrorListenersInjectable from "./api/setup-on-api-errors.injectable";
-import { observable } from "mobx";
+import { observable, computed } from "mobx";
 import defaultShellInjectable from "./components/+preferences/default-shell.injectable";
 import appVersionInjectable from "../common/get-configuration-file-model/app-version/app-version.injectable";
 import provideInitialValuesForSyncBoxesInjectable from "./utils/sync-box/provide-initial-values-for-sync-boxes.injectable";
@@ -59,6 +59,8 @@ import goForwardInjectable from "./components/layout/top-bar/go-forward.injectab
 import closeWindowInjectable from "./components/layout/top-bar/close-window.injectable";
 import maximizeWindowInjectable from "./components/layout/top-bar/maximize-window.injectable";
 import toggleMaximizeWindowInjectable from "./components/layout/top-bar/toggle-maximize-window.injectable";
+import commandContainerRootFrameChildComponentInjectable from "./components/command-palette/command-container-root-frame-child-component.injectable";
+import type { HotbarStore } from "../common/hotbars/store";
 
 export const getDiForUnitTesting = (opts: { doGeneralOverrides?: boolean } = {}) => {
   const {
@@ -103,6 +105,12 @@ export const getDiForUnitTesting = (opts: { doGeneralOverrides?: boolean } = {})
 
     di.override(lensResourcesDirInjectable, () => "/irrelevant");
 
+    di.override(commandContainerRootFrameChildComponentInjectable, () => ({
+      Component: () => null,
+      id: "command-container",
+      shouldRender: computed(() => false),
+    }));
+
     di.override(watchHistoryStateInjectable, () => () => () => {});
 
     di.override(openAppContextMenuInjectable, () => () => {});
@@ -126,7 +134,11 @@ export const getDiForUnitTesting = (opts: { doGeneralOverrides?: boolean } = {})
     // eslint-disable-next-line unused-imports/no-unused-vars-ts
     di.override(extensionsStoreInjectable, () => ({ isEnabled: ({ id, isBundled }) => false }) as ExtensionsStore);
 
-    di.override(hotbarStoreInjectable, () => ({}));
+    di.override(hotbarStoreInjectable, () => ({
+      getActive: () => ({ name: "some-hotbar", items: [] }),
+      getDisplayIndex: () => "0",
+    }) as unknown as HotbarStore);
+
 
     di.override(fileSystemProvisionerStoreInjectable, () => ({}) as FileSystemProvisionerStore);