From 6425d30f9c2cbc30b5be8257a56749fbd145ef82 Mon Sep 17 00:00:00 2001 From: Orlando Valverde Date: Wed, 13 Nov 2024 17:42:10 -0600 Subject: [PATCH 1/4] frontend: Improve breadcrumbs UX --- .../packages/core/src/AppProvider/index.tsx | 2 +- .../core/src/WorkflowLayout/index.tsx | 16 ++++----- .../src/utils/generateBreadcrumbsEntries.tsx | 36 +++++++++++++------ 3 files changed, 33 insertions(+), 21 deletions(-) diff --git a/frontend/packages/core/src/AppProvider/index.tsx b/frontend/packages/core/src/AppProvider/index.tsx index 0ea18eb2fc..fa4626212e 100644 --- a/frontend/packages/core/src/AppProvider/index.tsx +++ b/frontend/packages/core/src/AppProvider/index.tsx @@ -217,7 +217,7 @@ const ClutchApp = ({ : workflow.displayName; const workflowLayoutProps: LayoutProps = { - workflow, + workflowsInPath: workflows.filter(w => w.path === workflow.path), title: heading, subtitle: route.description, variant: diff --git a/frontend/packages/core/src/WorkflowLayout/index.tsx b/frontend/packages/core/src/WorkflowLayout/index.tsx index 3fb0f8bd53..1cc04dec2b 100644 --- a/frontend/packages/core/src/WorkflowLayout/index.tsx +++ b/frontend/packages/core/src/WorkflowLayout/index.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { matchPath, useParams } from "react-router-dom"; +import { useParams } from "react-router-dom"; import type { Interpolation } from "@emotion/styled"; import type { CSSObject, Theme } from "@mui/material"; import { alpha } from "@mui/material"; @@ -14,7 +14,7 @@ import { generateBreadcrumbsEntries } from "../utils"; export type LayoutVariant = "standard" | "wizard"; export type LayoutProps = { - workflow: Workflow; + workflowsInPath: Array; variant?: LayoutVariant | null; title?: string | ((params: Record) => string); subtitle?: string; @@ -88,7 +88,7 @@ const Subtitle = styled(Typography)(({ theme }: { theme: Theme }) => ({ })); const WorkflowLayout = ({ - workflow, + workflowsInPath, variant = null, title = null, subtitle = null, @@ -99,22 +99,18 @@ const WorkflowLayout = ({ const params = useParams(); const location = useLocation(); + const entries = generateBreadcrumbsEntries(workflowsInPath, location); + if (variant === null) { return <>{children}; } - const workflowPaths = workflow.routes.map(({ path }) => `/${workflow.path}/${path}`); - const breadcrumbsEntries = generateBreadcrumbsEntries( - location, - url => !!workflowPaths.find(path => !!matchPath({ path }, url)) - ); - return ( {!hideHeader && ( - + {!breadcrumbsOnly && (title || subtitle) && ( diff --git a/frontend/packages/core/src/utils/generateBreadcrumbsEntries.tsx b/frontend/packages/core/src/utils/generateBreadcrumbsEntries.tsx index 099e1e213b..9e2bf7cbe4 100644 --- a/frontend/packages/core/src/utils/generateBreadcrumbsEntries.tsx +++ b/frontend/packages/core/src/utils/generateBreadcrumbsEntries.tsx @@ -1,23 +1,39 @@ -import type { Location } from "react-router-dom"; +import { Location, matchPath } from "react-router-dom"; +import type { Workflow } from "../AppProvider/workflow"; import type { BreadcrumbEntry } from "../Breadcrumbs"; -const generateBreadcrumbsEntries = (location: Location, validateUrl: (url: string) => boolean) => { +const HOME_ENTRY = { label: "Home", url: "/" }; + +const generateBreadcrumbsEntries = (workflowsInPath: Array, location: Location) => { + const firstWorkflow = workflowsInPath[0]; + const allRoutes = workflowsInPath.flatMap(w => w.routes); + const fullPaths = allRoutes.map(({ path }) => `/${firstWorkflow.path}/${path}`); + const labels = decodeURIComponent(location.pathname) .split("/") .slice(1, location.pathname.endsWith("/") ? -1 : undefined); - const entries: Array = [{ label: "Home", url: "/" }].concat( - labels.map((label, index) => { - let url = `/${labels.slice(0, index + 1).join("/")}`; + const entries: Array = [HOME_ENTRY].concat( + labels.map((defaultLabel, index) => { + const nextIndex = index + 1; + const url = `/${labels.slice(0, nextIndex).join("/")}`; - if (!validateUrl(url)) { - url = undefined; - } + const path = fullPaths.find(p => !!matchPath(p, url)); + const route = path + ? allRoutes.find(r => + r.path.startsWith("/") + ? r.path + : `/${r.path}` === `/${path.split("/").slice(2).join("/")}` + ) + : null; return { - label, - url, + label: + route?.displayName || + (allRoutes.length === 1 && firstWorkflow.displayName) || + defaultLabel, + url: !!path && labels.length !== nextIndex ? url : null, }; }) ); From 2fa6db025bf9a5eed901067198403303a8ea09a1 Mon Sep 17 00:00:00 2001 From: Orlando Valverde Date: Wed, 13 Nov 2024 18:21:23 -0600 Subject: [PATCH 2/4] Update layout props in workflow config --- frontend/packages/core/src/AppProvider/workflow.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frontend/packages/core/src/AppProvider/workflow.tsx b/frontend/packages/core/src/AppProvider/workflow.tsx index 9ae6d29957..82a5d61287 100644 --- a/frontend/packages/core/src/AppProvider/workflow.tsx +++ b/frontend/packages/core/src/AppProvider/workflow.tsx @@ -55,7 +55,7 @@ interface WorkflowLayoutConfiguration { /** * (Optional) property to pass the defined layout properties to all of its defined routes */ - defaultLayoutProps?: Omit; + defaultLayoutProps?: Omit; } export interface Workflow @@ -105,7 +105,7 @@ export interface Route { /** * (Optional) property to define layout properties for a single route */ - layoutProps?: Omit; + layoutProps?: Omit; } export interface ConfiguredRoute extends Route { From a3fa8e24f3fc4b173e504f3346471dc5af64a939 Mon Sep 17 00:00:00 2001 From: Orlando Valverde Date: Fri, 15 Nov 2024 10:18:45 -0600 Subject: [PATCH 3/4] Fix issues stated in code review --- .../core/src/WorkflowLayout/index.tsx | 3 +- .../src/utils/generateBreadcrumbsEntries.tsx | 35 ++++++++++++++----- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/frontend/packages/core/src/WorkflowLayout/index.tsx b/frontend/packages/core/src/WorkflowLayout/index.tsx index 1cc04dec2b..63c3c86052 100644 --- a/frontend/packages/core/src/WorkflowLayout/index.tsx +++ b/frontend/packages/core/src/WorkflowLayout/index.tsx @@ -1,12 +1,11 @@ import React from "react"; -import { useParams } from "react-router-dom"; import type { Interpolation } from "@emotion/styled"; import type { CSSObject, Theme } from "@mui/material"; import { alpha } from "@mui/material"; import type { Workflow } from "../AppProvider/workflow"; import Breadcrumbs from "../Breadcrumbs"; -import { useLocation } from "../navigation"; +import { useLocation, useParams } from "../navigation"; import styled from "../styled"; import { Typography } from "../typography"; import { generateBreadcrumbsEntries } from "../utils"; diff --git a/frontend/packages/core/src/utils/generateBreadcrumbsEntries.tsx b/frontend/packages/core/src/utils/generateBreadcrumbsEntries.tsx index 9e2bf7cbe4..9e491338c3 100644 --- a/frontend/packages/core/src/utils/generateBreadcrumbsEntries.tsx +++ b/frontend/packages/core/src/utils/generateBreadcrumbsEntries.tsx @@ -6,34 +6,51 @@ import type { BreadcrumbEntry } from "../Breadcrumbs"; const HOME_ENTRY = { label: "Home", url: "/" }; const generateBreadcrumbsEntries = (workflowsInPath: Array, location: Location) => { + // The first workflow in the will contain + // the same path and displayName as the others const firstWorkflow = workflowsInPath[0]; + + if (!firstWorkflow) { + return [HOME_ENTRY]; + } + + // Get a single level list of the routes available const allRoutes = workflowsInPath.flatMap(w => w.routes); + + // Add to every item in the routes list the workflow path prefix const fullPaths = allRoutes.map(({ path }) => `/${firstWorkflow.path}/${path}`); - const labels = decodeURIComponent(location.pathname) + // Generate a list of path segments from the location + const pathSegments = decodeURIComponent(location.pathname) .split("/") - .slice(1, location.pathname.endsWith("/") ? -1 : undefined); + .slice(1, location.pathname.endsWith("/") ? -1 : undefined); // in case of a trailing `/` const entries: Array = [HOME_ENTRY].concat( - labels.map((defaultLabel, index) => { + pathSegments.map((segment, index) => { const nextIndex = index + 1; - const url = `/${labels.slice(0, nextIndex).join("/")}`; + const url = `/${pathSegments.slice(0, nextIndex).join("/")}`; const path = fullPaths.find(p => !!matchPath(p, url)); + + // If there is a matched path, it's used to find the route that contains its displayName const route = path ? allRoutes.find(r => r.path.startsWith("/") ? r.path - : `/${r.path}` === `/${path.split("/").slice(2).join("/")}` + : // Done in case of an empty path or missing a facing `/` + `/${r.path}` === `/${path.split("/").slice(2).join("/")}` ) : null; return { + // For the label: + // - Prioritize the display name + // - Handle the case of a single route with an unusual long name + // - Default to the path segment label: - route?.displayName || - (allRoutes.length === 1 && firstWorkflow.displayName) || - defaultLabel, - url: !!path && labels.length !== nextIndex ? url : null, + route?.displayName || (allRoutes.length === 1 && firstWorkflow.displayName) || segment, + // Set a null url if there is no path or for the last segment + url: !!path && pathSegments.length !== nextIndex ? url : null, }; }) ); From 50eef9b53acb2196016187c4a59bd825df8adbf1 Mon Sep 17 00:00:00 2001 From: Orlando Valverde Date: Fri, 15 Nov 2024 10:20:15 -0600 Subject: [PATCH 4/4] Change wording in comment --- frontend/packages/core/src/utils/generateBreadcrumbsEntries.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/packages/core/src/utils/generateBreadcrumbsEntries.tsx b/frontend/packages/core/src/utils/generateBreadcrumbsEntries.tsx index 9e491338c3..e2d0887b01 100644 --- a/frontend/packages/core/src/utils/generateBreadcrumbsEntries.tsx +++ b/frontend/packages/core/src/utils/generateBreadcrumbsEntries.tsx @@ -37,7 +37,7 @@ const generateBreadcrumbsEntries = (workflowsInPath: Array, location: ? allRoutes.find(r => r.path.startsWith("/") ? r.path - : // Done in case of an empty path or missing a facing `/` + : // Done in case of an empty path or missing a leading `/` `/${r.path}` === `/${path.split("/").slice(2).join("/")}` ) : null;