Make non-shown application menu items not break composite structure

This was made apparent by adding related unit tests for all known environments. Co-authored-by: Janne Savolainen <janne.savolainen@live.fi> Signed-off-by: Iku-turso <mikko.aspiala@gmail.com>
2025-05-20 05:10:56 +00:00 · 2022-10-24 13:29:35 +03:00 · 2022-10-24 13:29:35 +03:00 · b4f48a30cc
commit b4f48a30cc
parent 7048683c14
4 changed files with 128 additions and 18 deletions
--- a/src/features/application-menu/snapshots/application-menu.test.ts.snap
+++ b/src/features/application-menu/snapshots/application-menu.test.ts.snap
@ -1,6 +1,46 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP

-exports[`application-menu given enough time passes populates application menu 1`] = `
+exports[`application-menu, given environment is 'linux' given enough time passes populates application menu 1`] = `
+Array [
+  "root",
+  "root -> file",
+  "root -> file -> add-cluster",
+  "root -> file -> navigate-to-preferences",
+  "root -> file -> navigate-to-extensions",
+  "root -> file -> quit",
+  "root -> edit",
+  "root -> edit -> undo",
+  "root -> edit -> redo",
+  "root -> edit -> separator-1-in-edit",
+  "root -> edit -> cut",
+  "root -> edit -> copy",
+  "root -> edit -> paste",
+  "root -> edit -> delete",
+  "root -> edit -> separator-2-in-edit",
+  "root -> edit -> selectAll",
+  "root -> view",
+  "root -> view -> navigate-to-catalog",
+  "root -> view -> open-command-palette",
+  "root -> view -> separator-1-for-view",
+  "root -> view -> go-back",
+  "root -> view -> go-forward",
+  "root -> view -> reload",
+  "root -> view -> toggle-dev-tools",
+  "root -> view -> separator-2-for-view",
+  "root -> view -> reset-zoom",
+  "root -> view -> zoom-in",
+  "root -> view -> zoom-out",
+  "root -> view -> separator-3-for-view",
+  "root -> view -> toggle-full-screen",
+  "root -> help",
+  "root -> help -> navigate-to-welcome",
+  "root -> help -> open-documentation",
+  "root -> help -> open-support",
+  "root -> help -> about",
+]
+`;
+
+exports[`application-menu, given environment is 'mac' given enough time passes populates application menu 1`] = `
 Array [
  "root",
  "root -> mac",
@ -50,3 +90,43 @@ Array [
  "root -> help -> open-support",
 ]
 `;
+
+exports[`application-menu, given environment is 'win' given enough time passes populates application menu 1`] = `
+Array [
+  "root",
+  "root -> file",
+  "root -> file -> add-cluster",
+  "root -> file -> navigate-to-preferences",
+  "root -> file -> navigate-to-extensions",
+  "root -> file -> quit",
+  "root -> edit",
+  "root -> edit -> undo",
+  "root -> edit -> redo",
+  "root -> edit -> separator-1-in-edit",
+  "root -> edit -> cut",
+  "root -> edit -> copy",
+  "root -> edit -> paste",
+  "root -> edit -> delete",
+  "root -> edit -> separator-2-in-edit",
+  "root -> edit -> selectAll",
+  "root -> view",
+  "root -> view -> navigate-to-catalog",
+  "root -> view -> open-command-palette",
+  "root -> view -> separator-1-for-view",
+  "root -> view -> go-back",
+  "root -> view -> go-forward",
+  "root -> view -> reload",
+  "root -> view -> toggle-dev-tools",
+  "root -> view -> separator-2-for-view",
+  "root -> view -> reset-zoom",
+  "root -> view -> zoom-in",
+  "root -> view -> zoom-out",
+  "root -> view -> separator-3-for-view",
+  "root -> view -> toggle-full-screen",
+  "root -> help",
+  "root -> help -> navigate-to-welcome",
+  "root -> help -> open-documentation",
+  "root -> help -> open-support",
+  "root -> help -> about",
+]
+`;
--- a/src/features/application-menu/application-menu.test.ts
+++ b/src/features/application-menu/application-menu.test.ts
@ -7,8 +7,11 @@ import { getApplicationBuilder } from "../../renderer/components/test-utils/get-
 import populateApplicationMenuInjectable from "./main/populate-application-menu.injectable";
 import { advanceFakeTime, useFakeTime } from "../../common/test-utils/use-fake-time";
 import { getCompositePaths } from "../../common/utils/composite/get-composite-paths/get-composite-paths";
+import isMacInjectable from "../../common/vars/is-mac.injectable";
+import isWindowsInjectable from "../../common/vars/is-windows.injectable";
+import isLinuxInjectable from "../../common/vars/is-linux.injectable";

-describe("application-menu", () => {
+describe.each(["win", "mac", "linux"])("application-menu, given environment is '%s'", (environment) => {
  let builder: ApplicationBuilder;
  let populateApplicationMenuMock: jest.Mock;

@ -20,6 +23,26 @@ describe("application-menu", () => {
    builder = getApplicationBuilder();

    builder.beforeApplicationStart((mainDi) => {
+      switch (environment) {
+        case "mac":
+          mainDi.override(isMacInjectable, () => true);
+          mainDi.override(isWindowsInjectable, () => false);
+          mainDi.override(isLinuxInjectable, () => false);
+          break;
+
+        case "win":
+          mainDi.override(isMacInjectable, () => false);
+          mainDi.override(isWindowsInjectable, () => true);
+          mainDi.override(isLinuxInjectable, () => false);
+          break;
+
+        case "linux":
+          mainDi.override(isMacInjectable, () => false);
+          mainDi.override(isWindowsInjectable, () => false);
+          mainDi.override(isLinuxInjectable, () => true);
+          break;
+      }
+
      mainDi.override(
        populateApplicationMenuInjectable,
        () => populateApplicationMenuMock,
@ -50,7 +73,7 @@ describe("application-menu", () => {
    });

    it("populates application menu", () => {
-      expect(applicationMenuPaths).toMatchSnapshot();
+      expect(applicationMenuPaths.map(x => x.join(" -> "))).toMatchSnapshot();
    });
  });
 });
--- a/src/features/application-menu/main/application-menu-item-composite.injectable.ts
+++ b/src/features/application-menu/main/application-menu-item-composite.injectable.ts
@ -5,26 +5,27 @@
 import { getInjectable } from "@ogre-tools/injectable";
 import applicationMenuItemsInjectable from "./application-menu-items.injectable";
 import type { Composite } from "../../../common/utils/composite/get-composite/get-composite";
-import getComposite from "../../../common/utils/composite/get-composite/get-composite";
+import getCompositeFor from "../../../common/utils/composite/get-composite/get-composite";
 import { computed } from "mobx";
 import { pipeline } from "@ogre-tools/fp";
 import type { ApplicationMenuItemTypes } from "./menu-items/application-menu-item-injection-token";
-import loggerInjectable from "../../../common/logger.injectable";
 import type { RootComposite } from "../../../common/utils/composite/interfaces";
 import type { Discriminable } from "../../../common/utils/composable-responsibilities/discriminable/discriminable";
 import type { Orderable } from "../../../common/utils/composable-responsibilities/orderable/orderable";
+import logErrorInjectable from "../../../common/log-error.injectable";
+import { sortBy } from "lodash/fp";
+import { isShown } from "../../../common/utils/composable-responsibilities/showable/showable";

-export type MenuItemRoot =
-  & Discriminable<"root">
-  & RootComposite<"root">
-  & Orderable;
+export type MenuItemRoot = Discriminable<"root"> &
+  RootComposite<"root"> &
+  Orderable;

 const applicationMenuItemCompositeInjectable = getInjectable({
  id: "application-menu-item-composite",

  instantiate: (di) => {
    const menuItems = di.inject(applicationMenuItemsInjectable);
-    const logger = di.inject(loggerInjectable);
+    const logError = di.inject(logErrorInjectable);

    return computed((): Composite<ApplicationMenuItemTypes | MenuItemRoot> => {
      const items = menuItems.get();
@ -41,12 +42,21 @@ const applicationMenuItemCompositeInjectable = getInjectable({
          ...items,
        ],

-        (x) => getComposite({
-          source: x,
+        getCompositeFor({
+          getId: (x) => x.id,
+          getParentId: (x) => x.parentId,
+          transformChildren: (children) =>
+            pipeline(
+              children,
+              sortBy((child) => child.orderNumber),
+              (children) => children.filter(child => isShown(child)),
+            ),

          handleMissingParentIds: ({ missingParentIds }) => {
-            logger.error(
-              `[MENU]: cannot render menu item for missing parentIds: "${missingParentIds.join('", "')}"`,
+            logError(
+              `[MENU]: cannot render menu item for missing parentIds: "${missingParentIds.join(
+                '", "',
+              )}"`,
            );
          },
        }),
--- a/src/features/application-menu/main/application-menu-items.injectable.ts
+++ b/src/features/application-menu/main/application-menu-items.injectable.ts
@ -7,7 +7,6 @@ import type { MenuItemConstructorOptions } from "electron";
 import { computed } from "mobx";
 import applicationMenuItemInjectionToken from "./menu-items/application-menu-item-injection-token";
 import { computedInjectManyInjectable } from "@ogre-tools/injectable-extension-for-mobx";
-import { isShown } from "../../../common/utils/composable-responsibilities/showable/showable";

 export interface MenuItemOpts extends MenuItemConstructorOptions {
  submenu?: MenuItemConstructorOptions[];
@ -20,9 +19,7 @@ const applicationMenuItemsInjectable = getInjectable({
    const computedInjectMany = di.inject(computedInjectManyInjectable);

    return computed(() =>
-      computedInjectMany(applicationMenuItemInjectionToken)
-        .get()
-        .filter(isShown),
+      computedInjectMany(applicationMenuItemInjectionToken).get(),
    );
  },
 });