Fix stale UI state in extension preferences

Co-authored-by: Alex Andreev <alex.andreev.email@gmail.com> Signed-off-by: Janne Savolainen <janne.savolainen@live.fi>
2025-05-20 05:10:56 +00:00 · 2022-06-21 17:14:05 +03:00 · 2022-06-21 17:14:05 +03:00 · e2e0fd3860
commit e2e0fd3860
parent 4e15279894
6 changed files with 249 additions and 50 deletions
--- a/src/behaviours/preferences/snapshots/navigation-to-extension-specific-preferences.test.tsx.snap
+++ b/src/behaviours/preferences/snapshots/navigation-to-extension-specific-preferences.test.tsx.snap
@ -2155,6 +2155,165 @@ exports[`preferences - navigation to extension specific preferences given in pre
 </div>
 `;

+exports[`preferences - navigation to extension specific preferences given in preferences, when rendered when extension with specific preferences is enabled when navigating to extension preferences using navigation when extension is disabled renders 1`] = `
+<body>
+  <div>
+    <div
+      class="StatusBar"
+    >
+      <div
+        class="leftSide"
+      />
+      <div
+        class="rightSide"
+      />
+    </div>
+    <div
+      class="SettingLayout showNavigation Preferences"
+      data-testid="extension-preferences-page"
+    >
+      <nav
+        class="sidebarRegion"
+      >
+        <div
+          class="sidebar"
+        >
+          <div
+            class="Tabs flex column"
+          >
+            <div
+              class="header"
+            >
+              Preferences
+            </div>
+            <div
+              class="Tab flex gaps align-center"
+              data-testid="tab-link-for-application"
+              role="tab"
+              tabindex="0"
+            >
+              <div
+                class="label"
+              >
+                App
+              </div>
+            </div>
+            <div
+              class="Tab flex gaps align-center"
+              data-testid="tab-link-for-proxy"
+              role="tab"
+              tabindex="0"
+            >
+              <div
+                class="label"
+              >
+                Proxy
+              </div>
+            </div>
+            <div
+              class="Tab flex gaps align-center"
+              data-testid="tab-link-for-kubernetes"
+              role="tab"
+              tabindex="0"
+            >
+              <div
+                class="label"
+              >
+                Kubernetes
+              </div>
+            </div>
+            <div
+              class="Tab flex gaps align-center"
+              data-testid="tab-link-for-editor"
+              role="tab"
+              tabindex="0"
+            >
+              <div
+                class="label"
+              >
+                Editor
+              </div>
+            </div>
+            <div
+              class="Tab flex gaps align-center"
+              data-testid="tab-link-for-terminal"
+              role="tab"
+              tabindex="0"
+            >
+              <div
+                class="label"
+              >
+                Terminal
+              </div>
+            </div>
+          </div>
+        </div>
+      </nav>
+      <div
+        class="contentRegion"
+        id="ScrollSpyRoot"
+      >
+        <div
+          class="content"
+        >
+          <section
+            id="extensions"
+          >
+            <h2>
+               
+              preferences
+            </h2>
+            <div
+              class="flex items-center"
+              data-testid="error-for-extension-not-being-present"
+            >
+              No extension found
+            </div>
+          </section>
+        </div>
+        <div
+          class="toolsRegion"
+        >
+          <div
+            class="fixed top-[60px]"
+          >
+            <div
+              data-testid="close-preferences"
+            >
+              <div
+                aria-label="Close"
+                class="closeButton"
+                role="button"
+              >
+                <i
+                  class="Icon icon material focusable"
+                >
+                  <span
+                    class="icon"
+                    data-icon-name="close"
+                  >
+                    close
+                  </span>
+                </i>
+              </div>
+              <div
+                aria-hidden="true"
+                class="esc"
+              >
+                ESC
+              </div>
+            </div>
+          </div>
+        </div>
+      </div>
+    </div>
+    <div
+      class="Notifications flex column align-flex-end"
+    />
+  </div>
+</body>
+`;
+
 exports[`preferences - navigation to extension specific preferences when navigating to extension specific tab renders 1`] = `
 <div>
  <div
--- a/src/behaviours/preferences/navigation-to-extension-specific-preferences.test.tsx
+++ b/src/behaviours/preferences/navigation-to-extension-specific-preferences.test.tsx
@ -7,12 +7,13 @@ import type { ApplicationBuilder } from "../../renderer/components/test-utils/ge
 import { getApplicationBuilder } from "../../renderer/components/test-utils/get-application-builder";
 import React from "react";
 import "@testing-library/jest-dom/extend-expect";
-import type { FakeExtensionData } from "../../renderer/components/test-utils/get-renderer-extension-fake";
+import type { FakeExtensionData, TestExtension } from "../../renderer/components/test-utils/get-renderer-extension-fake";
 import { getRendererExtensionFakeFor } from "../../renderer/components/test-utils/get-renderer-extension-fake";
 import type { DiContainer } from "@ogre-tools/injectable";
 import { getDiForUnitTesting } from "../../renderer/getDiForUnitTesting";
 import extensionPreferencesRouteInjectable from "../../common/front-end-routing/routes/preferences/extension/extension-preferences-route.injectable";

+
 describe("preferences - navigation to extension specific preferences", () => {
  let applicationBuilder: ApplicationBuilder;

@ -98,9 +99,14 @@ describe("preferences - navigation to extension specific preferences", () => {
    });

    describe("when extension with specific preferences is enabled", () => {
+      let testExtension: TestExtension;
+
      beforeEach(() => {
        const getRendererExtensionFake = getRendererExtensionFakeFor(applicationBuilder);
-        const testExtension = getRendererExtensionFake(extensionStubWithExtensionSpecificPreferenceItems);
+
+        testExtension = getRendererExtensionFake(
+          extensionStubWithExtensionSpecificPreferenceItems,
+        );

        applicationBuilder.extensions.renderer.enable(testExtension);
      });
@ -159,6 +165,26 @@ describe("preferences - navigation to extension specific preferences", () => {

          expect(actual).toHaveClass("active");
        });
+
+        describe("when extension is disabled", () => {
+          beforeEach(() => {
+            applicationBuilder.extensions.renderer.disable(testExtension);
+          });
+
+          it("renders", () => {
+            expect(rendered.baseElement).toMatchSnapshot();
+          });
+
+          it("shows the error message about extension not being present", () => {
+            expect(rendered.getByTestId("error-for-extension-not-being-present")).toBeInTheDocument();
+          });
+
+          it("when extension is enabled again, does not show the error message anymore", () => {
+            applicationBuilder.extensions.renderer.enable(testExtension);
+
+            expect(rendered.queryByTestId("error-for-extension-not-being-present")).not.toBeInTheDocument();
+          });
+        });
      });
    });

@ -292,10 +318,10 @@ describe("preferences - navigation to extension specific preferences", () => {
  describe("when navigating to extension specific tab", () => {
    let rendered: RenderResult;
    let di: DiContainer;
-    
+
    beforeEach(async () => {
      di = getDiForUnitTesting({ doGeneralOverrides: true });
-      
+
      const getRendererExtensionFake = getRendererExtensionFakeFor(applicationBuilder);
      const extension = getRendererExtensionFake(extensionStubWithWithSameRegisteredTab);
      const otherExtension = getRendererExtensionFake(extensionUsingSomeoneElseTab);
@ -306,10 +332,10 @@ describe("preferences - navigation to extension specific preferences", () => {
          extensionId: "duplicated-tab-page-id",
          tabId: "metrics-extension-tab",
        }};
-        
+
        applicationBuilder.preferences.navigateTo(extensionRoute, params);
      });
-      
+
      await applicationBuilder.extensions.renderer.enable(extension, otherExtension);
      rendered = await applicationBuilder.render();
    });
@ -330,10 +356,10 @@ describe("preferences - navigation to extension specific preferences", () => {
  describe("when navigating to someone else extension specific tab", () => {
    let rendered: RenderResult;
    let di: DiContainer;
-    
+
    beforeEach(async () => {
      di = getDiForUnitTesting({ doGeneralOverrides: true });
-      
+
      const getRendererExtensionFake = getRendererExtensionFakeFor(applicationBuilder);
      const extension = getRendererExtensionFake(extensionStubWithWithSameRegisteredTab);
      const extensionUsingOtherTab = getRendererExtensionFake(extensionUsingSomeoneElseTab);
@ -344,10 +370,10 @@ describe("preferences - navigation to extension specific preferences", () => {
          extensionId: "extension-using-someone-else-tab-id",
          tabId: "metrics-extension-tab",
        }};
-        
+
        applicationBuilder.preferences.navigateTo(extensionRoute, params);
      });
-      
+
      await applicationBuilder.extensions.renderer.enable(extension, extensionUsingOtherTab);
      rendered = await applicationBuilder.render();
    });
--- a/src/renderer/components/+preferences/extensions.tsx
+++ b/src/renderer/components/+preferences/extensions.tsx
@ -4,6 +4,7 @@
 */

 import { withInjectables } from "@ogre-tools/injectable-react";
+import type { IComputedValue } from "mobx";
 import { observer } from "mobx-react";
 import React from "react";
 import type { RegisteredAppPreference } from "./app-preferences/app-preference-registration";
@ -12,44 +13,50 @@ import { ExtensionSettings } from "./extension-settings";
 import { Preferences } from "./preferences";

 interface Dependencies {
-  preferenceItems: RegisteredAppPreference[];
-  extensionName?: string;
+  model: IComputedValue<{
+    preferenceItems: RegisteredAppPreference[];
+    extensionName?: string;
+  }>;
 }

-const NonInjectedExtensions = ({ preferenceItems, extensionName }: Dependencies) => (
-  <Preferences data-testid="extension-preferences-page">
-    <section id="extensions">
-      <h2>
-        {extensionName}
-        {" "}
-        preferences
-      </h2>
-      {!extensionName && (
-        <div className="flex items-center">No extension found</div>
-      )}
-      {preferenceItems.map((preferenceItem, index) => (
-        <ExtensionSettings
-          key={`${preferenceItem.id}-${index}`}
-          setting={preferenceItem}
-          size="small"
-          data-testid={`extension-preference-item-for-${preferenceItem.id}`}
-        />
-      ))}
-    </section>
-  </Preferences>
-);
+const NonInjectedExtensions = ({ model }: Dependencies) => {
+  const { extensionName, preferenceItems } = model.get();
+
+  return (
+    <Preferences data-testid="extension-preferences-page">
+      <section id="extensions">
+        <h2>
+          {extensionName}
+          {" "}
+          preferences
+        </h2>
+        {!extensionName && (
+          <div
+            className="flex items-center"
+            data-testid="error-for-extension-not-being-present"
+          >
+            No extension found
+          </div>
+        )}
+        {preferenceItems.map((preferenceItem, index) => (
+          <ExtensionSettings
+            key={`${preferenceItem.id}-${index}`}
+            setting={preferenceItem}
+            size="small"
+            data-testid={`extension-preference-item-for-${preferenceItem.id}`}
+          />
+        ))}
+      </section>
+    </Preferences>
+  );
+};

 export const Extensions = withInjectables<Dependencies>(
  observer(NonInjectedExtensions),

  {
-    getProps: (di) => {
-      const { preferenceItems, extensionName } = di.inject(extensionPreferencesModelInjectable).get();
-
-      return {
-        preferenceItems,
-        extensionName,
-      };
-    },
+    getProps: (di) => ({
+      model: di.inject(extensionPreferencesModelInjectable),
+    }),
  },
 );
--- a/src/renderer/components/+preferences/preferences-navigation/extensions-nav-group.tsx
+++ b/src/renderer/components/+preferences/preferences-navigation/extensions-nav-group.tsx
@ -9,12 +9,13 @@ import type { PreferenceNavigationItem } from "./preference-navigation-items.inj
 import { Icon } from "../../icon";
 import { PreferencesNavigationTab } from "./preference-navigation-tab";
 import preferenceNavigationItemsForGroupInjectable from "./preference-navigation-items-for-group.injectable";
+import { observer } from "mobx-react";

 interface Dependencies {
  navigationItems: IComputedValue<PreferenceNavigationItem[]>;
 }

-function NonInjectedExtensionsNavGroup(props: Dependencies) {
+const NonInjectedExtensionsNavGroup = observer((props: Dependencies) => {
  if (!props.navigationItems.get().length) {
    return null;
  }
@ -42,7 +43,7 @@ function NonInjectedExtensionsNavGroup(props: Dependencies) {
      </div>
    </div>
  );
-}
+});

 export const ExtensionsNavGroup = withInjectables<Dependencies>(
  NonInjectedExtensionsNavGroup,
--- a/src/renderer/components/+preferences/preferences-navigation/general-nav-group.tsx
+++ b/src/renderer/components/+preferences/preferences-navigation/general-nav-group.tsx
@ -8,12 +8,13 @@ import type { IComputedValue } from "mobx";
 import type { PreferenceNavigationItem } from "./preference-navigation-items.injectable";
 import { PreferencesNavigationTab } from "./preference-navigation-tab";
 import preferenceNavigationItemsForGroupInjectable from "./preference-navigation-items-for-group.injectable";
+import { observer } from "mobx-react";

 interface Dependencies {
  navigationItems: IComputedValue<PreferenceNavigationItem[]>;
 }

-function NonInjectedGeneralNavGroup(props: Dependencies) {
+const NonInjectedGeneralNavGroup = observer((props: Dependencies) => {
  if (!props.navigationItems.get().length) {
    return null;
  }
@ -31,7 +32,7 @@ function NonInjectedGeneralNavGroup(props: Dependencies) {
      ))}
    </React.Fragment>
  );
-}
+});

 export const GeneralNavGroup = withInjectables<Dependencies>(
  NonInjectedGeneralNavGroup,
--- a/src/renderer/components/+preferences/preferences-navigation/preference-navigation-items.injectable.ts
+++ b/src/renderer/components/+preferences/preferences-navigation/preference-navigation-items.injectable.ts
@ -4,6 +4,7 @@
 */
 import { pipeline } from "@ogre-tools/fp";
 import { getInjectable, getInjectionToken } from "@ogre-tools/injectable";
+import { computedInjectManyInjectable } from "@ogre-tools/injectable-extension-for-mobx";
 import { filter, orderBy } from "lodash/fp";
 import type { IComputedValue } from "mobx";
 import { computed } from "mobx";
@ -26,14 +27,18 @@ export interface PreferenceNavigationItem {
 const preferenceNavigationItemsInjectable = getInjectable({
  id: "preference-navigation-items",

-  instantiate: (di) =>
-    computed((): PreferenceNavigationItem[] =>
+  instantiate: (di) => {
+    const computedInjectMany = di.inject(computedInjectManyInjectable);
+    const navigationItems = computedInjectMany(preferenceNavigationItemInjectionToken);
+
+    return computed((): PreferenceNavigationItem[] =>
      pipeline(
-        di.injectMany(preferenceNavigationItemInjectionToken),
+        navigationItems.get(),
        filter((item) => !!item.isVisible.get()),
        (items) => orderBy([(item) => item.orderNumber], ["asc"], items),
      ),
-    ),
+    );
+  },
 });

 export default preferenceNavigationItemsInjectable;