From e2dc678cf000c21dea9f4e46779681bfb791e600 Mon Sep 17 00:00:00 2001 From: Birk Johansson Date: Mon, 11 Nov 2024 14:22:54 +0100 Subject: [PATCH] refactor(breadcrumbs): simplify and refactor breadcrumbs (#434) * refactor: simplify and improve breadcrumbitem * fix: fix tests * fix: improve route handle type --- i18n/en.pot | 31 +++++++++++- src/app/layout/Breadcrumb.module.css | 18 ++++--- src/app/layout/Breadcrumb.spec.tsx | 45 ++++++++++-------- src/app/layout/Breadcrumb.tsx | 45 +++++++++++------- src/app/routes/Router.tsx | 71 ++++++++++++++++++++++------ src/app/routes/types.ts | 10 ++-- 6 files changed, 154 insertions(+), 66 deletions(-) diff --git a/i18n/en.pot b/i18n/en.pot index 17fadb7c..baf333f8 100644 --- a/i18n/en.pot +++ b/i18n/en.pot @@ -5,8 +5,8 @@ msgstr "" "Content-Type: text/plain; charset=utf-8\n" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=(n != 1)\n" -"POT-Creation-Date: 2024-11-05T07:23:15.731Z\n" -"PO-Revision-Date: 2024-11-05T07:23:15.732Z\n" +"POT-Creation-Date: 2024-11-05T14:43:19.245Z\n" +"PO-Revision-Date: 2024-11-05T14:43:19.245Z\n" msgid "schemas" msgstr "schemas" @@ -33,6 +33,12 @@ msgstr "Not found" msgid "The page you are looking for does not exist." msgstr "The page you are looking for does not exist." +msgid "New {{modelName}}" +msgstr "New {{modelName}}" + +msgid "Edit {{modelName}}" +msgstr "Edit {{modelName}}" + msgid "Metadata Overview" msgstr "Metadata Overview" @@ -1154,6 +1160,27 @@ msgstr "" "included. PHU will still be available for the PHU level, but not included " "in the aggregations to the levels above." +msgid "Setup" +msgstr "Setup" + +msgid "Data" +msgstr "Data" + +msgid "Data Elements" +msgstr "Data Elements" + +msgid "Periods" +msgstr "Periods" + +msgid "Organisation Units" +msgstr "Organisation Units" + +msgid "Form" +msgstr "Form" + +msgid "Advanced" +msgstr "Advanced" + msgid "Longitude" msgstr "Longitude" diff --git a/src/app/layout/Breadcrumb.module.css b/src/app/layout/Breadcrumb.module.css index 0dc3757f..7a812c2a 100644 --- a/src/app/layout/Breadcrumb.module.css +++ b/src/app/layout/Breadcrumb.module.css @@ -2,28 +2,32 @@ margin-bottom: 8px; } -.breadcrumbItem { - color: var(--colors-blue600); - text-decoration: none; +.breadCrumbItem { font-size: 14px; line-height: 18px; } -span.breadcrumbItem { +.breadcrumbItemLink { + composes: breadCrumbItem; + color: var(--colors-blue600); + text-decoration: none; +} + +span.breadcrumbItemLink { cursor: pointer; } -.breadcrumbItem:hover { +.breadcrumbItemLink:hover { text-decoration: underline; color: var(--colors-blue700); } -.breadcrumbItem:active { +.breadcrumbItemLink:active { text-decoration: underline; color: var(--colors-blue900); } -.breadcrumbItem:focus { +.breadcrumbItemLink:focus { outline: 1px solid var(--colors-blue600); } diff --git a/src/app/layout/Breadcrumb.spec.tsx b/src/app/layout/Breadcrumb.spec.tsx index 03ce6aa6..98eedc42 100644 --- a/src/app/layout/Breadcrumb.spec.tsx +++ b/src/app/layout/Breadcrumb.spec.tsx @@ -2,7 +2,12 @@ import '@testing-library/jest-dom' import { configure, render } from '@testing-library/react' import React from 'react' import { useMatches, HashRouter } from 'react-router-dom' -import { OVERVIEW_SECTIONS, SECTIONS_MAP } from '../../lib' +import { + getOverviewPath, + getSectionPath, + OVERVIEW_SECTIONS, + SECTIONS_MAP, +} from '../../lib' import { MatchRouteHandle, RouteHandle } from '../routes/types' import { Breadcrumbs, BreadcrumbItem } from './Breadcrumb' @@ -34,9 +39,12 @@ beforeEach(() => { }) describe('BreadcrumbItem', () => { - it('should render a link based on the section title', () => { + it('should render a link based on the given label', () => { const { getByRole } = render( - , + , { wrapper: HashRouter } ) @@ -46,9 +54,12 @@ describe('BreadcrumbItem', () => { expect(DataElementLink).toHaveAttribute('href', '#/dataElements') }) - it('should render a link to overview-section using plural title if overview-section', () => { + it('should render a correct link to overview-section using plural title if overview-section', () => { const { getByRole } = render( - , + , { wrapper: HashRouter } ) @@ -60,20 +71,6 @@ describe('BreadcrumbItem', () => { '#/overview/dataElements' ) }) - it('should render a link with label-prop instead of section title if provided', () => { - const { getByRole } = render( - , - { wrapper: HashRouter } - ) - - const DataElementLink = getByRole('link') - expect(DataElementLink).toBeDefined() - expect(DataElementLink).toHaveTextContent('Data elements') - expect(DataElementLink).toHaveAttribute('href', '#/dataElements') - }) }) describe('Breadcrumbs', () => { it('should render crumb components in handle', () => { @@ -97,13 +94,19 @@ describe('Breadcrumbs', () => { mockHandle({ crumb: () => ( ), }), mockHandle({ crumb: () => ( - + ), }), ]) diff --git a/src/app/layout/Breadcrumb.tsx b/src/app/layout/Breadcrumb.tsx index 890b05fa..9d128b89 100644 --- a/src/app/layout/Breadcrumb.tsx +++ b/src/app/layout/Breadcrumb.tsx @@ -1,36 +1,44 @@ import React from 'react' -import { Link, useMatches } from 'react-router-dom' -import { - Section, - isOverviewSection, - getSectionPath, - getOverviewPath, - useToWithSearchState, -} from '../../lib' +import { Link, To, useLocation, useMatches, matchPath } from 'react-router-dom' +import { useToWithSearchState } from '../../lib' import type { MatchRouteHandle } from '../routes/types' import css from './Breadcrumb.module.css' const BreadcrumbSeparator = () => / type BreadcrumbItemProps = { - section: Section - label?: string + label: string + to: To } -export const BreadcrumbItem = ({ section, label }: BreadcrumbItemProps) => { - const isOverview = isOverviewSection(section) - const link = isOverview ? getOverviewPath(section) : getSectionPath(section) - const to = useToWithSearchState(`/${link}`) +export const BreadcrumbItem = ({ label, to }: BreadcrumbItemProps) => { + const resolvedTo = useToWithSearchState(to) + const currentLoc = useLocation() - label = label ?? isOverview ? section.titlePlural : section.title + if (resolvedTo.pathname) { + const match = matchPath(resolvedTo.pathname, currentLoc.pathname) + if (match?.pattern.end) { + return + } + } return ( - + {label} ) } +/** Component that is used for "End links", where the current route is the end of the path + * and thus should not be a link */ +export const BreadCrumbEndItem = ({ label }: { label: string }) => ( + {label} +) + export const Breadcrumbs = () => { const matches = useMatches() as MatchRouteHandle[] @@ -38,7 +46,10 @@ export const Breadcrumbs = () => { .filter((match) => match.handle?.crumb) .map((match) => ( - {match.handle?.crumb?.()} + {match.handle?.crumb?.({ + params: match.params, + pathname: match.pathname, + })} )) diff --git a/src/app/routes/Router.tsx b/src/app/routes/Router.tsx index df1ddd24..02e4b343 100644 --- a/src/app/routes/Router.tsx +++ b/src/app/routes/Router.tsx @@ -1,3 +1,4 @@ +import i18n from '@dhis2/d2-i18n' import React from 'react' import { createHashRouter, @@ -20,13 +21,14 @@ import { isModuleNotFoundError, isValidUid, routePaths, + getOverviewPath, } from '../../lib' import { OverviewSection } from '../../types' import { Layout, Breadcrumbs, BreadcrumbItem } from '../layout' import { CheckAuthorityForSection } from './CheckAuthorityForSection' import { DefaultErrorRoute } from './DefaultErrorRoute' import { LegacyAppRedirect } from './LegacyAppRedirect' - +import { RouteHandle } from './types' // This loads all the overview routes in the same chunk since they resolve to the same promise // see https://reactrouter.com/en/main/route/lazy#multiple-routes-in-a-single-file // Overviews are small, and the AllOverview would load all the other overviews anyway, @@ -109,14 +111,22 @@ const schemaSectionRoutes = Object.values(SCHEMA_SECTIONS).map((section) => ( ( - - ), - }} + handle={ + { + section, + crumb: () => ( + + ), + } satisfies RouteHandle + } element={ <> @@ -126,21 +136,52 @@ const schemaSectionRoutes = Object.values(SCHEMA_SECTIONS).map((section) => ( > , - }} + handle={ + { + hideSidebar: true, + crumb: (matchInfo) => ( + + ), + } satisfies RouteHandle + } > {!sectionsNoNewRoute.has(section) && ( ( + + ), + } satisfies RouteHandle + } /> )} }> ( + + ), + } satisfies RouteHandle + } lazy={createSectionLazyRouteFunction(section, 'Edit')} /> @@ -175,7 +216,7 @@ const routes = createRoutesFromElements( section.componentName, section )} - handle={{ section }} + handle={{ section } satisfies RouteHandle} /> ))} diff --git a/src/app/routes/types.ts b/src/app/routes/types.ts index e280b69f..62429dd6 100644 --- a/src/app/routes/types.ts +++ b/src/app/routes/types.ts @@ -1,17 +1,19 @@ -import type { useMatches } from 'react-router-dom' -import type { ModelSection } from '../../types' +import type { UIMatch, useMatches } from 'react-router-dom' +import type { ModelSection, OverviewSection } from '../../types' // utility type to type a match with a handle-property returned from useMatches // since handle is unknown, we need to cast it to the correct type type MatchWithHandle = ReturnType[number] & { handle?: THandle } +export type BreadCrumbMatchInfo = Pick + // common type for possible handle-properties used in Route export type RouteHandle = { hideSidebar?: boolean - section?: ModelSection + section?: ModelSection | OverviewSection showFooter?: boolean - crumb?: () => React.ReactNode + crumb?: (matchInfo: BreadCrumbMatchInfo) => React.ReactNode } export type MatchRouteHandle = MatchWithHandle