Rebuild App Settings to be nested inside app screen (#3676)

* Rebuild App Settings to be nested inside app screen * Remove memoization * Update src/apps/components/AppPage/AppPageNav.tsx Co-authored-by: Michał Droń <droniu@droniu.dev> --------- Co-authored-by: Michał Droń <droniu@droniu.dev>
2023-05-22 08:35:35 +02:00 · 2023-05-22 08:35:35 +02:00 · 8f58efcd50
commit 8f58efcd50
parent ea5b6a80a5
15 changed files with 60 additions and 135 deletions
--- a/locale/defaultMessages.json
+++ b/locale/defaultMessages.json
@ -35,10 +35,6 @@
    "context": "order shipping method name",
    "string": "Shipping"
  },
-  "+FWlRD": {
-    "context": "button",
-    "string": "Open app"
-  },
  "+HuipK": {
    "context": "variant sku",
    "string": "SKU {sku}"
@ -1812,10 +1808,6 @@
    "context": "header",
    "string": "Activity"
  },
-  "BYGJ/j": {
-    "context": "button label",
-    "string": "Settings"
-  },
  "BYTvv/": {
    "context": "Dry run events unavailable",
    "string": "The following events from provided query are currently not available for dry run:"
@ -3298,6 +3290,10 @@
    "context": "button",
    "string": "Create order"
  },
+  "LwX0Ug": {
+    "context": "Button with Manage app label",
+    "string": "Manage app"
+  },
  "Lx1ima": {
    "context": "button",
    "string": "Upload image"
--- a/src/apps/components/AppDetailsPage/AppDetailsPage.stories.tsx
+++ b/src/apps/components/AppDetailsPage/AppDetailsPage.stories.tsx
@ -6,7 +6,6 @@ import AppDetailsPage, { AppDetailsPageProps } from "./AppDetailsPage";
 const props: AppDetailsPageProps = {
  data: appDetails,
  loading: false,
-  navigateToApp: () => undefined,
  onAppActivateOpen: () => undefined,
  onAppDeactivateOpen: () => undefined,
  onAppDeleteOpen: () => undefined,
--- a/src/apps/components/AppDetailsPage/AppDetailsPage.test.tsx
+++ b/src/apps/components/AppDetailsPage/AppDetailsPage.test.tsx
@ -38,7 +38,6 @@ beforeEach(() => {
 describe("Apps AppDetailsPage", () => {
  it("displays app details when app data passed", () => {
    // Arrange
-    const navigateToApp = jest.fn();
    const onAppActivateOpen = jest.fn();
    const onAppDeactivateOpen = jest.fn();
    const onAppDeleteOpen = jest.fn();
@ -48,7 +47,6 @@ describe("Apps AppDetailsPage", () => {
      <AppDetailsPage
        data={appDetails}
        loading={false}
-        navigateToApp={navigateToApp}
        onAppActivateOpen={onAppActivateOpen}
        onAppDeactivateOpen={onAppDeactivateOpen}
        onAppDeleteOpen={onAppDeleteOpen}
@ -58,7 +56,6 @@ describe("Apps AppDetailsPage", () => {
    // Assert
    expect(mockHeader).toHaveBeenCalledWith({
      data: appDetails,
-      navigateToApp,
      onAppActivateOpen,
      onAppDeactivateOpen,
      onAppDeleteOpen,
--- a/src/apps/components/AppDetailsPage/AppDetailsPage.tsx
+++ b/src/apps/components/AppDetailsPage/AppDetailsPage.tsx
@ -10,7 +10,6 @@ import PermissionsCard from "./PermissionsCard";
 export interface AppDetailsPageProps {
  loading: boolean;
  data: AppQuery["app"];
-  navigateToApp: () => void;
  onAppActivateOpen: () => void;
  onAppDeactivateOpen: () => void;
  onAppDeleteOpen: () => void;
@ -19,7 +18,6 @@ export interface AppDetailsPageProps {
 export const AppDetailsPage: React.FC<AppDetailsPageProps> = ({
  data,
  loading,
-  navigateToApp,
  onAppActivateOpen,
  onAppDeactivateOpen,
  onAppDeleteOpen,
@ -27,7 +25,6 @@ export const AppDetailsPage: React.FC<AppDetailsPageProps> = ({
  <>
    <Header
      data={data}
-      navigateToApp={navigateToApp}
      onAppActivateOpen={onAppActivateOpen}
      onAppDeactivateOpen={onAppDeactivateOpen}
      onAppDeleteOpen={onAppDeleteOpen}
--- a/src/apps/components/AppDetailsPage/Header.test.tsx
+++ b/src/apps/components/AppDetailsPage/Header.test.tsx
@ -6,7 +6,6 @@ import Header from "./Header";

 const mockHeaderOptions = jest.fn();
 const mockTopNav = jest.fn();
-const mockButton = jest.fn();

 jest.mock("@dashboard/components/AppLayout/TopNav", () => ({
  TopNav: props => {
@ -14,12 +13,6 @@ jest.mock("@dashboard/components/AppLayout/TopNav", () => ({
    return <>{props.children}</>;
  },
 }));
-jest.mock("@saleor/macaw-ui/next", () => ({
-  Button: props => {
-    mockButton(props);
-    return <>{props.children}</>;
-  },
-}));
 jest.mock("../DeactivatedText", () => () => "deactivated");
 jest.mock("react-intl", () => ({
  useIntl: jest.fn(() => ({
@ -36,13 +29,11 @@ jest.mock("./HeaderOptions", () => props => {
 beforeEach(() => {
  mockHeaderOptions.mockClear();
  mockTopNav.mockClear();
-  mockButton.mockClear();
 });

 describe("Apps AppDetailsPage Header", () => {
  it("displays app details options when active app data passed", () => {
    // Arrange
-    const navigateToApp = jest.fn();
    const onAppActivateOpen = jest.fn();
    const onAppDeactivateOpen = jest.fn();
    const onAppDeleteOpen = jest.fn();
@ -51,7 +42,6 @@ describe("Apps AppDetailsPage Header", () => {
    render(
      <Header
        data={appDetails}
-        navigateToApp={navigateToApp}
        onAppActivateOpen={onAppActivateOpen}
        onAppDeactivateOpen={onAppDeactivateOpen}
        onAppDeleteOpen={onAppDeleteOpen}
@ -66,18 +56,12 @@ describe("Apps AppDetailsPage Header", () => {
      onAppDeactivateOpen,
      onAppDeleteOpen,
    });
-    expect(mockButton).toHaveBeenCalledWith(
-      expect.objectContaining({
-        onClick: navigateToApp,
-      }),
-    );
    expect(mockTopNav).toHaveBeenCalled();
    expect(title.container).toHaveTextContent(appDetails.name as string);
  });

  it("displays app details options when inactive app data passed", () => {
    // Arrange
-    const navigateToApp = jest.fn();
    const onAppActivateOpen = jest.fn();
    const onAppDeactivateOpen = jest.fn();
    const onAppDeleteOpen = jest.fn();
@ -86,7 +70,6 @@ describe("Apps AppDetailsPage Header", () => {
    render(
      <Header
        data={{ ...appDetails, isActive: false }}
-        navigateToApp={navigateToApp}
        onAppActivateOpen={onAppActivateOpen}
        onAppDeactivateOpen={onAppDeactivateOpen}
        onAppDeleteOpen={onAppDeleteOpen}
@ -101,11 +84,6 @@ describe("Apps AppDetailsPage Header", () => {
      onAppDeactivateOpen,
      onAppDeleteOpen,
    });
-    expect(mockButton).toHaveBeenCalledWith(
-      expect.objectContaining({
-        onClick: navigateToApp,
-      }),
-    );
    expect(mockTopNav).toHaveBeenCalled();
    expect(title.container).toHaveTextContent(`${appDetails.name} deactivated`);
  });
--- a/src/apps/components/AppDetailsPage/Header.tsx
+++ b/src/apps/components/AppDetailsPage/Header.tsx
@ -1,17 +1,13 @@
-import { AppPaths } from "@dashboard/apps/urls";
+import { AppUrls } from "@dashboard/apps/urls";
 import { TopNav } from "@dashboard/components/AppLayout/TopNav";
 import { AppQuery } from "@dashboard/graphql";
-import { Button } from "@saleor/macaw-ui/next";
 import React from "react";
-import { FormattedMessage } from "react-intl";

 import DeactivatedText from "../DeactivatedText";
 import HeaderOptions from "./HeaderOptions";
-import messages from "./messages";

 interface HeaderProps {
  data: AppQuery["app"];
-  navigateToApp: () => void;
  onAppActivateOpen: () => void;
  onAppDeactivateOpen: () => void;
  onAppDeleteOpen: () => void;
@ -19,30 +15,32 @@ interface HeaderProps {

 const Header: React.FC<HeaderProps> = ({
  data,
-  navigateToApp,
  onAppActivateOpen,
  onAppDeactivateOpen,
  onAppDeleteOpen,
-}) => (
-  <>
-    <TopNav
-      href={AppPaths.appListPath}
-      title={
-        <>
-          {data?.name} {!data?.isActive && <DeactivatedText />}
-        </>
-      }
-    >
-      <Button onClick={navigateToApp} variant="primary" data-tc="open-app">
-        <FormattedMessage {...messages.openApp} />
-      </Button>
-    </TopNav>
-    <HeaderOptions
-      data={data}
-      onAppActivateOpen={onAppActivateOpen}
-      onAppDeactivateOpen={onAppDeactivateOpen}
-      onAppDeleteOpen={onAppDeleteOpen}
-    />
-  </>
-);
+}) => {
+  /**
+   * App is null with first render so fallback with HTML-safe fallback
+   */
+  const backButtonTarget = data?.id ? AppUrls.resolveAppUrl(data.id) : "#";
+
+  return (
+    <>
+      <TopNav
+        href={backButtonTarget}
+        title={
+          <>
+            {data?.name} {!data?.isActive && <DeactivatedText />}
+          </>
+        }
+      />
+      <HeaderOptions
+        data={data}
+        onAppActivateOpen={onAppActivateOpen}
+        onAppDeactivateOpen={onAppDeactivateOpen}
+        onAppDeleteOpen={onAppDeleteOpen}
+      />
+    </>
+  );
+};
 export default Header;
--- a/src/apps/components/AppDetailsPage/messages.ts
+++ b/src/apps/components/AppDetailsPage/messages.ts
@ -1,11 +1,6 @@
 import { defineMessages } from "react-intl";

 export default defineMessages({
-  openApp: {
-    id: "+FWlRD",
-    defaultMessage: "Open app",
-    description: "button",
-  },
  supportLink: {
    id: "Nsk5WL",
    defaultMessage: "Get support",
--- a/src/apps/components/AppPage/AppPage.tsx
+++ b/src/apps/components/AppPage/AppPage.tsx
@ -25,6 +25,7 @@ export const AppPage: React.FC<AppPageProps> = ({
 }) => (
  <DetailPageLayout gridTemplateColumns={1} withSavebar={false}>
    <AppPageNav
+      appId={data.id}
      name={data?.name}
      supportUrl={data?.supportUrl}
      homepageUrl={data?.homepageUrl}
--- a/src/apps/components/AppPage/AppPageNav.tsx
+++ b/src/apps/components/AppPage/AppPageNav.tsx
@ -2,6 +2,7 @@ import { AppAvatar } from "@dashboard/apps/components/AppAvatar/AppAvatar";
 import { AppUrls } from "@dashboard/apps/urls";
 import { TopNavLink, TopNavWrapper } from "@dashboard/components/AppLayout";
 import { LinkState } from "@dashboard/components/Link";
+import useNavigator from "@dashboard/hooks/useNavigator";
 import { Box, Button, Text } from "@saleor/macaw-ui/next";
 import React from "react";
 import { FormattedMessage } from "react-intl";
@ -12,6 +13,7 @@ interface AppPageNavProps {
  supportUrl: string | undefined | null;
  homepageUrl: string | undefined | null;
  author: string | undefined | null;
+  appId: string;
 }

 export const AppPageNav: React.FC<AppPageNavProps> = ({
@ -19,9 +21,15 @@ export const AppPageNav: React.FC<AppPageNavProps> = ({
  supportUrl,
  homepageUrl,
  author,
+  appId,
 }) => {
  const location = useLocation<LinkState>();
  const goBackLink = location.state?.from ?? AppUrls.resolveAppListUrl();
+  const navigate = useNavigator();
+
+  const navigateToManageAppScreen = () => {
+    navigate(AppUrls.resolveAppDetailsUrl(appId));
+  };

  return (
    <TopNavWrapper>
@ -55,6 +63,18 @@ export const AppPageNav: React.FC<AppPageNavProps> = ({
        </Box>
      </Box>
      <Box display="flex" gap={4}>
+        <Button
+          whiteSpace="nowrap"
+          variant="secondary"
+          onClick={navigateToManageAppScreen}
+          data-test-id="app-settings-button"
+        >
+          <FormattedMessage
+            defaultMessage="Manage app"
+            id="LwX0Ug"
+            description="Button with Manage app label"
+          />
+        </Button>
        {supportUrl && (
          <Button
            variant="secondary"
--- a/src/apps/components/InstalledAppListRow/InstalledAppListRow.test.tsx
+++ b/src/apps/components/InstalledAppListRow/InstalledAppListRow.test.tsx
@ -4,7 +4,6 @@ import { InstalledApp } from "@dashboard/apps/types";
 import { getAppsConfig } from "@dashboard/config";
 import Wrapper from "@test/wrapper";
 import { render, screen } from "@testing-library/react";
-import userEvent from "@testing-library/user-event";
 import React from "react";
 import { MemoryRouter as Router } from "react-router-dom";

@ -29,7 +28,6 @@ const Component = ({
 describe("Apps InstalledAppListRow", () => {
  it("displays app details when basic app data passed", () => {
    // Arrange
-    const openAppSettings = jest.fn();
    const removeAppInstallation = jest.fn();
    const retryAppInstallation = jest.fn();
    render(
@ -39,7 +37,6 @@ describe("Apps InstalledAppListRow", () => {
          isExternal: false,
        }}
        context={{
-          openAppSettings,
          removeAppInstallation,
          retryAppInstallation,
        }}
@ -67,9 +64,9 @@ describe("Apps InstalledAppListRow", () => {

  it("displays external label when app is external", () => {
    // Arrange
-    const openAppSettings = jest.fn();
    const removeAppInstallation = jest.fn();
    const retryAppInstallation = jest.fn();
+
    render(
      <Component
        data={{
@ -77,7 +74,6 @@ describe("Apps InstalledAppListRow", () => {
          isExternal: true,
        }}
        context={{
-          openAppSettings,
          removeAppInstallation,
          retryAppInstallation,
        }}
@ -91,10 +87,10 @@ describe("Apps InstalledAppListRow", () => {

  it("displays tunnnel label when app is served via tunnnel", () => {
    // Arrange
-    const openAppSettings = jest.fn();
    const removeAppInstallation = jest.fn();
    const retryAppInstallation = jest.fn();
    const AppsConfig = getAppsConfig();
+
    render(
      <Component
        data={{
@ -106,7 +102,6 @@ describe("Apps InstalledAppListRow", () => {
          isExternal: false,
        }}
        context={{
-          openAppSettings,
          removeAppInstallation,
          retryAppInstallation,
        }}
@ -117,33 +112,4 @@ describe("Apps InstalledAppListRow", () => {
    // Assert
    expect(tunnelLabel).toBeTruthy();
  });
-
-  it("calls handlers when app data passed and buttons clicked", async () => {
-    // Arrange
-    const openAppSettings = jest.fn();
-    const removeAppInstallation = jest.fn();
-    const retryAppInstallation = jest.fn();
-    render(
-      <Component
-        data={{
-          app: activeApp,
-          isExternal: false,
-        }}
-        context={{
-          openAppSettings,
-          removeAppInstallation,
-          retryAppInstallation,
-        }}
-      />,
-    );
-    const user = userEvent.setup();
-    const settingsButton = screen.getByTestId("app-settings-button");
-
-    // Act
-    await user.click(settingsButton);
-
-    // Assert
-    expect(openAppSettings).toHaveBeenCalledWith(activeApp.id);
-    expect(openAppSettings).toHaveBeenCalledTimes(1);
-  });
 });
--- a/src/apps/components/InstalledAppListRow/InstalledAppListRow.tsx
+++ b/src/apps/components/InstalledAppListRow/InstalledAppListRow.tsx
@ -1,18 +1,9 @@
-import { useAppListContext } from "@dashboard/apps/context";
 import { appsMessages } from "@dashboard/apps/messages";
 import { InstalledApp } from "@dashboard/apps/types";
 import { AppUrls } from "@dashboard/apps/urls";
 import { isAppInTunnel } from "@dashboard/apps/utils";
 import Link from "@dashboard/components/Link";
-import { TableButtonWrapper } from "@dashboard/components/TableButtonWrapper/TableButtonWrapper";
-import {
-  Box,
-  Button,
-  Chip,
-  List,
-  sprinkles,
-  Text,
-} from "@saleor/macaw-ui/next";
+import { Box, Chip, List, sprinkles, Text } from "@saleor/macaw-ui/next";
 import React from "react";
 import { FormattedMessage, useIntl } from "react-intl";
 import { useLocation } from "react-router";
@ -25,7 +16,7 @@ import { messages } from "./messages";
 export const InstalledAppListRow: React.FC<InstalledApp> = props => {
  const { app, isExternal, logo } = props;
  const intl = useIntl();
-  const { openAppSettings } = useAppListContext();
+
  const location = useLocation();

  return (
@ -44,7 +35,11 @@ export const InstalledAppListRow: React.FC<InstalledApp> = props => {
        justifyContent="space-between"
        flexDirection="row"
        flexWrap="wrap"
-        backgroundColor={!app.isActive ? "surfaceNeutralSubdued" : undefined}
+        transition={"ease"}
+        backgroundColor={{
+          default: !app.isActive ? "surfaceNeutralSubdued" : undefined,
+          hover: "surfaceNeutralSubdued",
+        }}
        cursor={app.isActive ? "pointer" : "not-allowed"}
      >
        <Box
@ -102,15 +97,6 @@ export const InstalledAppListRow: React.FC<InstalledApp> = props => {
            )}
            <AppPermissions permissions={app.permissions} />
          </Box>
-          <TableButtonWrapper>
-            <Button
-              variant="secondary"
-              onClick={() => openAppSettings(app.id)}
-              data-test-id="app-settings-button"
-            >
-              <FormattedMessage {...messages.settings} />
-            </Button>
-          </TableButtonWrapper>
        </Box>
      </List.Item>
    </Link>
--- a/src/apps/components/InstalledAppListRow/messages.ts
+++ b/src/apps/components/InstalledAppListRow/messages.ts
@ -6,11 +6,6 @@ export const messages = defineMessages({
    defaultMessage: "Tunnel - development",
    description: "label",
  },
-  settings: {
-    id: "BYGJ/j",
-    defaultMessage: "Settings",
-    description: "button label",
-  },
  appDisabled: {
    id: "7u9Ep7",
    defaultMessage: "Disabled",
--- a/src/apps/context.ts
+++ b/src/apps/context.ts
@ -4,7 +4,6 @@ import React from "react";
 export interface AppListContextValues {
  removeAppInstallation: (installationId: string) => void;
  retryAppInstallation: (installationId: string) => void;
-  openAppSettings: (appId: string) => void;
 }

 export const AppListContext = React.createContext<
--- a/src/apps/views/AppDetails/AppDetails.tsx
+++ b/src/apps/views/AppDetails/AppDetails.tsx
@ -154,7 +154,6 @@ export const AppDetails: React.FC<AppDetailsProps> = ({ id, params }) => {
      <AppDetailsPage
        data={data?.app || null}
        loading={loading}
-        navigateToApp={() => navigate(AppUrls.resolveAppUrl(id))}
        onAppActivateOpen={() => openModal("app-activate")}
        onAppDeactivateOpen={() => openModal("app-deactivate")}
        onAppDeleteOpen={() => openModal("app-delete")}
--- a/src/apps/views/AppList/AppList.tsx
+++ b/src/apps/views/AppList/AppList.tsx
@ -135,7 +135,6 @@ export const AppsList: React.FC<AppsListProps> = ({ params }) => {
    () => ({
      retryAppInstallation: handleAppInstallRetry,
      removeAppInstallation: id => openModal("app-installation-remove", { id }),
-      openAppSettings: id => navigate(AppUrls.resolveAppDetailsUrl(id)),
    }),
    [navigate, openModal],
  );