From a7763ee2d4d2a92d40120f11afa181d47cf88ca0 Mon Sep 17 00:00:00 2001 From: Shivaji Byrapaneni Date: Tue, 8 Sep 2026 10:12:29 +0100 Subject: [PATCH 1/6] feat(calm-hub-ui): loading states and link-based navigation Extracted from #3001 (OIDC + SCM backend support), which bundled these unrelated UI changes with the GitHub storage backend work. - Explore rail and mobile nav show loading state while counts resolve - Namespace/type in the section header render as links - Sparkline overflow fix (#2728) Original-PR: finos/architecture-as-code#3001 --- calm-hub-ui/src/hub/Hub.tsx | 2 + .../diagram-section/timeline/Sparkline.tsx | 5 +- .../timeline/TimelineBar.test.tsx | 11 ++-- .../explore-rail/ExploreRail.test.tsx | 20 ++++++- .../components/explore-rail/ExploreRail.tsx | 52 ++++++++++++------- .../section-header/SectionHeader.test.tsx | 8 ++- .../section-header/SectionHeader.tsx | 6 ++- .../tree-navigation/MobileNavMenu.test.tsx | 22 ++++++++ .../tree-navigation/MobileNavMenu.tsx | 11 ++-- 9 files changed, 102 insertions(+), 35 deletions(-) diff --git a/calm-hub-ui/src/hub/Hub.tsx b/calm-hub-ui/src/hub/Hub.tsx index a68f259611..ae8ee6ede4 100644 --- a/calm-hub-ui/src/hub/Hub.tsx +++ b/calm-hub-ui/src/hub/Hub.tsx @@ -465,6 +465,7 @@ export default function Hub() { setIsSidebarOpen(false)} /> ) : ( @@ -498,6 +499,7 @@ export default function Hub() { setIsMobileNavOpen(false)} /> diff --git a/calm-hub-ui/src/hub/components/diagram-section/timeline/Sparkline.tsx b/calm-hub-ui/src/hub/components/diagram-section/timeline/Sparkline.tsx index af69eaa55f..82b63e2eeb 100644 --- a/calm-hub-ui/src/hub/components/diagram-section/timeline/Sparkline.tsx +++ b/calm-hub-ui/src/hub/components/diagram-section/timeline/Sparkline.tsx @@ -151,12 +151,13 @@ export function Sparkline({ {/* Track row. A single-version resource has nothing to scrub, so the track is suppressed (the version pill already states what's shown). - overflow-hidden clips long labels at the track edge (#2728). */} + Individual labels are bounded by maxWidth + ellipsis (#2728); the + track itself uses overflow-visible so the last label isn't clipped. */} {!singleVersion && (
{/* Inner track wrapper inset 10px each side so dot percentages map directly */}
diff --git a/calm-hub-ui/src/hub/components/diagram-section/timeline/TimelineBar.test.tsx b/calm-hub-ui/src/hub/components/diagram-section/timeline/TimelineBar.test.tsx index 360e83d3e8..9f962268dd 100644 --- a/calm-hub-ui/src/hub/components/diagram-section/timeline/TimelineBar.test.tsx +++ b/calm-hub-ui/src/hub/components/diagram-section/timeline/TimelineBar.test.tsx @@ -257,13 +257,12 @@ describe('TimelineBar', () => { // #2728 — long moment names must not block the expand control or clip cards. describe('long moment names are bounded (#2728)', () => { - it('clips the collapsed sparkline track so labels cannot paint over the expand button', () => { + it('does not clip the sparkline track so edge labels remain fully visible', () => { renderBar(); - // The centre track is clipped so an overlong label can never overflow - // out to cover the statically-positioned expand button. - expect(screen.getByTestId('timeline-sparkline-track')).toHaveStyle({ - overflow: 'hidden', - }); + // Per-label maxWidth + ellipsis bounds individual labels (#2728); + // the track itself must NOT clip so the last label is not cut off. + const track = screen.getByTestId('timeline-sparkline-track'); + expect(track).not.toHaveStyle({ overflow: 'hidden' }); }); it('truncates each collapsed label with an ellipsis while keeping its full-name tooltip', () => { diff --git a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.test.tsx b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.test.tsx index c20af66e39..a002fca8c8 100644 --- a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.test.tsx +++ b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.test.tsx @@ -14,7 +14,7 @@ const domainCounts: DomainControlCount[] = [ { domain: 'compliance', controlCount: 0 }, ]; -const renderRail = (path = '/', onCollapse?: () => void) => +const renderRail = (path = '/', onCollapse?: () => void, loading?: boolean) => render( @@ -26,6 +26,7 @@ const renderRail = (path = '/', onCollapse?: () => void) => } @@ -91,4 +92,21 @@ describe('ExploreRail', () => { expect(onCollapse).toHaveBeenCalled(); await screen.findByRole('link', { name: /finos/ }); }); + + it('shows loading spinners instead of items when loading is true', () => { + renderRail('/', undefined, true); + const spinners = screen.getAllByClassName + ? document.querySelectorAll('.loading-spinner') + : screen.getByText('NAMESPACES').parentElement!.querySelectorAll('.loading-spinner'); + expect(spinners.length).toBeGreaterThanOrEqual(2); + expect(screen.queryByRole('link', { name: /finos/ })).not.toBeInTheDocument(); + expect(screen.queryByRole('link', { name: /security/ })).not.toBeInTheDocument(); + }); + + it('shows items instead of spinners when loading is false', async () => { + renderRail('/', undefined, false); + expect(await screen.findByRole('link', { name: /finos/ })).toBeInTheDocument(); + expect(screen.getByRole('link', { name: /security/ })).toBeInTheDocument(); + expect(document.querySelectorAll('.loading-spinner').length).toBe(0); + }); }); diff --git a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx index 800333aa51..769bfe562d 100644 --- a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx +++ b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx @@ -12,6 +12,8 @@ interface ExploreRailProps { namespaceCounts: NamespaceCounts[]; /** Per-domain control counts, fetched once by {@link Hub} and passed down. */ domainCounts: DomainControlCount[]; + /** True while the counts are still being fetched from the backend. */ + loading?: boolean; /** Collapse the rail (keeps the existing sidebar collapse affordance). */ onCollapse?: () => void; } @@ -28,7 +30,7 @@ type RailRouteParams = { ns?: string; domain?: string; namespace?: string }; * once there and shared), so this component takes them as props rather than * re-fetching them itself. */ -export function ExploreRail({ namespaceCounts, domainCounts, onCollapse }: ExploreRailProps) { +export function ExploreRail({ namespaceCounts, domainCounts, loading, onCollapse }: ExploreRailProps) { // `ns` comes from /namespace/:ns; on the detail route /:namespace/:type/:id/:version the // param is `namespace`. Fall back to it so the rail keeps its highlight during a detail session. const { ns, domain: activeDomain, namespace } = useParams(); @@ -82,28 +84,40 @@ export function ExploreRail({ namespaceCounts, domainCounts, onCollapse }: Explo
NAMESPACES
- {filteredNamespaces.map((nc) => ( - - ))} + {loading ? ( +
+ +
+ ) : ( + filteredNamespaces.map((nc) => ( + + )) + )}
CONTROL DOMAINS
- {domainCounts.map((dc) => ( - - ))} + {loading ? ( +
+ +
+ ) : ( + domainCounts.map((dc) => ( + + )) + )}
diff --git a/calm-hub-ui/src/hub/components/section-header/SectionHeader.test.tsx b/calm-hub-ui/src/hub/components/section-header/SectionHeader.test.tsx index 40d6326dda..b4aba41ec1 100644 --- a/calm-hub-ui/src/hub/components/section-header/SectionHeader.test.tsx +++ b/calm-hub-ui/src/hub/components/section-header/SectionHeader.test.tsx @@ -1,9 +1,15 @@ +import React from 'react'; import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { SectionHeader } from './SectionHeader.js'; +import { MemoryRouter } from 'react-router-dom'; +import { SectionHeader as SectionHeaderRaw } from './SectionHeader.js'; import { describe, it, expect, vi } from 'vitest'; import type { BreadcrumbItem } from '../../../model/calm.js'; +function SectionHeader(props: React.ComponentProps) { + return ; +} + describe('SectionHeader', () => { it('renders icon, namespace, id, and version', () => { const icon = Icon; diff --git a/calm-hub-ui/src/hub/components/section-header/SectionHeader.tsx b/calm-hub-ui/src/hub/components/section-header/SectionHeader.tsx index b08325b7a5..d4c667c9cc 100644 --- a/calm-hub-ui/src/hub/components/section-header/SectionHeader.tsx +++ b/calm-hub-ui/src/hub/components/section-header/SectionHeader.tsx @@ -1,4 +1,5 @@ import { ReactNode, useState } from 'react'; +import { Link } from 'react-router-dom'; import { IoCopyOutline, IoCheckmarkOutline, IoLinkOutline } from 'react-icons/io5'; import { BreadcrumbItem, isSlug } from '../../../model/calm.js'; import { BreadcrumbTrail } from './BreadcrumbTrail.js'; @@ -45,11 +46,12 @@ export function SectionHeader({ icon, namespace, id, version, typeSegment, right

{icon} {breadcrumbs && } - {namespace} + {namespace} {typeLabel && ( <> {' '} - / {typeLabel} + /{' '} + {typeLabel} )}{' '} /{' '} diff --git a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.test.tsx b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.test.tsx index 518362bb62..05d7be2b5d 100644 --- a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.test.tsx +++ b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.test.tsx @@ -185,4 +185,26 @@ describe('MobileNavMenu', () => { expect(await screen.findByText('traderx')).toBeInTheDocument(); expect(screen.queryByText('Architectures')).not.toBeInTheDocument(); }); + + it('shows a spinner at the root level when countsLoading is true', () => { + render( + + + + ); + const spinner = document.querySelector('.loading-spinner'); + expect(spinner).toBeInTheDocument(); + expect(screen.queryByText('Namespaces')).not.toBeInTheDocument(); + }); + + it('shows rows at the root level when countsLoading is false', () => { + render( + + + + ); + expect(document.querySelector('.loading-spinner')).not.toBeInTheDocument(); + expect(screen.getByText('Namespaces')).toBeInTheDocument(); + expect(screen.getByText('Control Domains')).toBeInTheDocument(); + }); }); diff --git a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx index 821dbbec26..ebc8c2325c 100644 --- a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx +++ b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx @@ -27,6 +27,8 @@ interface MobileNavMenuProps { namespaceCounts: NamespaceCounts[]; /** Per-domain control counts, fetched once by {@link Hub} and passed down. */ domainCounts: DomainControlCount[]; + /** True while the counts are still being fetched from the backend. */ + countsLoading?: boolean; /** Dismiss the menu (e.g. after a resource is chosen). */ onClose: () => void; } @@ -66,7 +68,7 @@ interface LeafItem { * {@link Hub} (fetched once and shared) and passed in as props rather than * re-fetched here. */ -export function MobileNavMenu({ namespaceCounts, domainCounts, onClose }: MobileNavMenuProps) { +export function MobileNavMenu({ namespaceCounts, domainCounts, countsLoading, onClose }: MobileNavMenuProps) { const navigate = useNavigate(); const params = useParams(); @@ -287,7 +289,8 @@ export function MobileNavMenu({ namespaceCounts, domainCounts, onClose }: Mobile } })(); - const isEmpty = !loading && rows.length === 0; + const showLoading = loading || (countsLoading && (view.level === 'root' || view.level === 'namespaces' || view.level === 'domains')); + const isEmpty = !showLoading && rows.length === 0; return (
@@ -309,7 +312,7 @@ export function MobileNavMenu({ namespaceCounts, domainCounts, onClose }: Mobile {!searching && (
    - {loading && ( + {showLoading && (
    • @@ -317,7 +320,7 @@ export function MobileNavMenu({ namespaceCounts, domainCounts, onClose }: Mobile {isEmpty && (
  • Nothing here
    • )} - {!loading && + {!showLoading && rows.map((row) => (
diff --git a/calm-hub-ui/src/hub/components/diagram-section/timeline/Sparkline.tsx b/calm-hub-ui/src/hub/components/diagram-section/timeline/Sparkline.tsx index 82b63e2eeb..4c44b650a0 100644 --- a/calm-hub-ui/src/hub/components/diagram-section/timeline/Sparkline.tsx +++ b/calm-hub-ui/src/hub/components/diagram-section/timeline/Sparkline.tsx @@ -24,6 +24,9 @@ interface ContextMenuState { y: number; } +/** Label box width; edge labels are anchored so this can never leave the card (#2728). */ +const LABEL_WIDTH = 120; + /** * Collapsed timeline strip — a "Browse versions" header (with the current-version * pill) above a sparkline of version dots with title + date below each. Single @@ -151,8 +154,9 @@ export function Sparkline({ {/* Track row. A single-version resource has nothing to scrub, so the track is suppressed (the version pill already states what's shown). - Individual labels are bounded by maxWidth + ellipsis (#2728); the - track itself uses overflow-visible so the last label isn't clipped. */} + Labels are bounded by LABEL_WIDTH + ellipsis, and the first/last are + anchored to their column so they stay inside DiagramSection's + overflow-hidden card rather than being sliced (#2728). */} {!singleVersion && (
- {moment.label} -
- {moment.validFrom && (
- {moment.validFrom} + {moment.label}
- )} + {moment.validFrom && ( +
+ {moment.validFrom} +
+ )} +

); })} diff --git a/calm-hub-ui/src/hub/components/diagram-section/timeline/TimelineBar.test.tsx b/calm-hub-ui/src/hub/components/diagram-section/timeline/TimelineBar.test.tsx index 9f962268dd..efec3ffd2d 100644 --- a/calm-hub-ui/src/hub/components/diagram-section/timeline/TimelineBar.test.tsx +++ b/calm-hub-ui/src/hub/components/diagram-section/timeline/TimelineBar.test.tsx @@ -257,12 +257,14 @@ describe('TimelineBar', () => { // #2728 — long moment names must not block the expand control or clip cards. describe('long moment names are bounded (#2728)', () => { - it('does not clip the sparkline track so edge labels remain fully visible', () => { + it('anchors the first and last collapsed labels to their column so they cannot leave the card', () => { renderBar(); - // Per-label maxWidth + ellipsis bounds individual labels (#2728); - // the track itself must NOT clip so the last label is not cut off. - const track = screen.getByTestId('timeline-sparkline-track'); - expect(track).not.toHaveStyle({ overflow: 'hidden' }); + // The first/last dots sit close to the card edge, so a centred label + // would overflow the ancestor card's overflow-hidden boundary and be + // sliced. Edge labels grow inward instead of centering (#2728). + expect(screen.getByText('1.0.0')).toHaveStyle({ textAlign: 'left' }); + expect(screen.getByText('1.5.0')).toHaveStyle({ textAlign: 'center' }); + expect(screen.getByText('2.0.0')).toHaveStyle({ textAlign: 'right' }); }); it('truncates each collapsed label with an ellipsis while keeping its full-name tooltip', () => { diff --git a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.test.tsx b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.test.tsx index a002fca8c8..8c2910570f 100644 --- a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.test.tsx +++ b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.test.tsx @@ -14,7 +14,13 @@ const domainCounts: DomainControlCount[] = [ { domain: 'compliance', controlCount: 0 }, ]; -const renderRail = (path = '/', onCollapse?: () => void, loading?: boolean) => +interface RenderRailOptions { + onCollapse?: () => void; + namespacesLoading?: boolean; + domainsLoading?: boolean; +} + +const renderRail = (path = '/', opts: RenderRailOptions = {}) => render( @@ -26,8 +32,9 @@ const renderRail = (path = '/', onCollapse?: () => void, loading?: boolean) => } /> @@ -87,26 +94,37 @@ describe('ExploreRail', () => { it('invokes onCollapse when the collapse button is clicked', async () => { const onCollapse = vi.fn(); - renderRail('/', onCollapse); + renderRail('/', { onCollapse }); fireEvent.click(screen.getByLabelText('Collapse sidebar')); expect(onCollapse).toHaveBeenCalled(); await screen.findByRole('link', { name: /finos/ }); }); - it('shows loading spinners instead of items when loading is true', () => { - renderRail('/', undefined, true); - const spinners = screen.getAllByClassName - ? document.querySelectorAll('.loading-spinner') - : screen.getByText('NAMESPACES').parentElement!.querySelectorAll('.loading-spinner'); - expect(spinners.length).toBeGreaterThanOrEqual(2); + it('shows a spinner in both sections while both are loading', () => { + renderRail('/', { namespacesLoading: true, domainsLoading: true }); + expect(screen.getAllByRole('status')).toHaveLength(2); expect(screen.queryByRole('link', { name: /finos/ })).not.toBeInTheDocument(); expect(screen.queryByRole('link', { name: /security/ })).not.toBeInTheDocument(); }); - it('shows items instead of spinners when loading is false', async () => { - renderRail('/', undefined, false); + it('resolves the namespaces section independently of a still-loading domains section', async () => { + renderRail('/', { namespacesLoading: false, domainsLoading: true }); + expect(await screen.findByRole('link', { name: /finos/ })).toBeInTheDocument(); + expect(screen.getByRole('status', { name: 'Loading control domains' })).toBeInTheDocument(); + expect(screen.queryByRole('link', { name: /security/ })).not.toBeInTheDocument(); + }); + + it('resolves the domains section independently of a still-loading namespaces section', async () => { + renderRail('/', { namespacesLoading: true, domainsLoading: false }); + expect(await screen.findByRole('link', { name: /security/ })).toBeInTheDocument(); + expect(screen.getByRole('status', { name: 'Loading namespaces' })).toBeInTheDocument(); + expect(screen.queryByRole('link', { name: /finos/ })).not.toBeInTheDocument(); + }); + + it('shows items instead of spinners once both sections finish loading', async () => { + renderRail('/', { namespacesLoading: false, domainsLoading: false }); expect(await screen.findByRole('link', { name: /finos/ })).toBeInTheDocument(); expect(screen.getByRole('link', { name: /security/ })).toBeInTheDocument(); - expect(document.querySelectorAll('.loading-spinner').length).toBe(0); + expect(screen.queryByRole('status')).not.toBeInTheDocument(); }); }); diff --git a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx index 769bfe562d..18de1e6c2c 100644 --- a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx +++ b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx @@ -1,4 +1,4 @@ -import { useMemo, useState } from 'react'; +import { ReactNode, useMemo, useState } from 'react'; import { useParams } from 'react-router-dom'; import { IoCompassOutline, IoChevronBackOutline } from 'react-icons/io5'; import { NamespaceCounts, DomainControlCount } from '../../../model/counts.js'; @@ -12,12 +12,26 @@ interface ExploreRailProps { namespaceCounts: NamespaceCounts[]; /** Per-domain control counts, fetched once by {@link Hub} and passed down. */ domainCounts: DomainControlCount[]; - /** True while the counts are still being fetched from the backend. */ - loading?: boolean; + /** True while the namespace counts are still being fetched. */ + namespacesLoading?: boolean; + /** True while the domain control counts are still being fetched. */ + domainsLoading?: boolean; /** Collapse the rail (keeps the existing sidebar collapse affordance). */ onCollapse?: () => void; } +function RailSpinner({ label }: { label: string }) { + return ( +
+ +
+ ); +} + +function RailEmpty({ children }: { children: ReactNode }) { + return
{children}
; +} + type RailRouteParams = { ns?: string; domain?: string; namespace?: string }; /** @@ -30,7 +44,13 @@ type RailRouteParams = { ns?: string; domain?: string; namespace?: string }; * once there and shared), so this component takes them as props rather than * re-fetching them itself. */ -export function ExploreRail({ namespaceCounts, domainCounts, loading, onCollapse }: ExploreRailProps) { +export function ExploreRail({ + namespaceCounts, + domainCounts, + namespacesLoading, + domainsLoading, + onCollapse, +}: ExploreRailProps) { // `ns` comes from /namespace/:ns; on the detail route /:namespace/:type/:id/:version the // param is `namespace`. Fall back to it so the rail keeps its highlight during a detail session. const { ns, domain: activeDomain, namespace } = useParams(); @@ -84,10 +104,10 @@ export function ExploreRail({ namespaceCounts, domainCounts, loading, onCollapse
NAMESPACES
- {loading ? ( -
- -
+ {namespacesLoading ? ( + + ) : filteredNamespaces.length === 0 ? ( + Nothing here ) : ( filteredNamespaces.map((nc) => ( CONTROL DOMAINS
- {loading ? ( -
- -
+ {domainsLoading ? ( + + ) : domainCounts.length === 0 ? ( + Nothing here ) : ( domainCounts.map((dc) => ( {' '} /{' '} - {typeLabel} + {typeLabel} )}{' '} /{' '} diff --git a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.test.tsx b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.test.tsx index 05d7be2b5d..9c623dc400 100644 --- a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.test.tsx +++ b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.test.tsx @@ -186,25 +186,33 @@ describe('MobileNavMenu', () => { expect(screen.queryByText('Architectures')).not.toBeInTheDocument(); }); - it('shows a spinner at the root level when countsLoading is true', () => { + it('shows the static root rows immediately, even while counts are still loading', () => { render( - + ); - const spinner = document.querySelector('.loading-spinner'); - expect(spinner).toBeInTheDocument(); - expect(screen.queryByText('Namespaces')).not.toBeInTheDocument(); + // The root rows are static labels, not derived from counts, so they must + // never be hidden behind a counts spinner. + expect(screen.queryByRole('status')).not.toBeInTheDocument(); + expect(screen.getByText('Namespaces')).toBeInTheDocument(); + expect(screen.getByText('Control Domains')).toBeInTheDocument(); }); - it('shows rows at the root level when countsLoading is false', () => { + it('shows a spinner only for the section whose own counts are still loading', async () => { render( - + ); - expect(document.querySelector('.loading-spinner')).not.toBeInTheDocument(); - expect(screen.getByText('Namespaces')).toBeInTheDocument(); - expect(screen.getByText('Control Domains')).toBeInTheDocument(); + // A single OR'd flag couldn't tell these two cases apart. + fireEvent.click(screen.getByText('Namespaces')); + expect(await screen.findByText('traderx')).toBeInTheDocument(); + expect(screen.queryByRole('status')).not.toBeInTheDocument(); + + fireEvent.click(screen.getByLabelText('Back')); + fireEvent.click(screen.getByText('Control Domains')); + expect(screen.getByRole('status')).toBeInTheDocument(); + expect(screen.queryByText('security')).not.toBeInTheDocument(); }); }); diff --git a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx index ebc8c2325c..68c30a1820 100644 --- a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx +++ b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx @@ -27,8 +27,10 @@ interface MobileNavMenuProps { namespaceCounts: NamespaceCounts[]; /** Per-domain control counts, fetched once by {@link Hub} and passed down. */ domainCounts: DomainControlCount[]; - /** True while the counts are still being fetched from the backend. */ - countsLoading?: boolean; + /** True while the namespace counts are still being fetched. */ + namespacesLoading?: boolean; + /** True while the domain control counts are still being fetched. */ + domainsLoading?: boolean; /** Dismiss the menu (e.g. after a resource is chosen). */ onClose: () => void; } @@ -68,7 +70,13 @@ interface LeafItem { * {@link Hub} (fetched once and shared) and passed in as props rather than * re-fetched here. */ -export function MobileNavMenu({ namespaceCounts, domainCounts, countsLoading, onClose }: MobileNavMenuProps) { +export function MobileNavMenu({ + namespaceCounts, + domainCounts, + namespacesLoading, + domainsLoading, + onClose, +}: MobileNavMenuProps) { const navigate = useNavigate(); const params = useParams(); @@ -79,7 +87,7 @@ export function MobileNavMenu({ namespaceCounts, domainCounts, countsLoading, on const [view, setView] = useState({ level: 'root' }); const [leafItems, setLeafItems] = useState([]); - const [loading, setLoading] = useState(false); + const [leafLoading, setLeafLoading] = useState(false); const [searching, setSearching] = useState(false); // Derive the namespace/domain lists from the counts Hub already fetched, rather than @@ -113,10 +121,10 @@ export function MobileNavMenu({ namespaceCounts, domainCounts, countsLoading, on (namespace: string, type: TypeInUI) => { setView({ level: 'resources', namespace, type }); setLeafItems([]); - setLoading(true); + setLeafLoading(true); const finish = (items: LeafItem[]) => { setLeafItems(items); - setLoading(false); + setLeafLoading(false); }; if (type === 'Interfaces') { interfaceService @@ -151,14 +159,14 @@ export function MobileNavMenu({ namespaceCounts, domainCounts, countsLoading, on (domain: string) => { setView({ level: 'controls', domain }); setLeafItems([]); - setLoading(true); + setLeafLoading(true); controlService .fetchControlsForDomain(domain) .then((controls: ControlDetail[]) => setLeafItems(controls.map((c) => ({ id: c.id.toString(), name: c.title ?? c.name }))) ) .catch(() => setLeafItems([])) - .finally(() => setLoading(false)); + .finally(() => setLeafLoading(false)); }, [controlService] ); @@ -289,7 +297,13 @@ export function MobileNavMenu({ namespaceCounts, domainCounts, countsLoading, on } })(); - const showLoading = loading || (countsLoading && (view.level === 'root' || view.level === 'namespaces' || view.level === 'domains')); + // The root rows ('Namespaces', 'Control Domains') are static labels, not + // count-derived, so they render immediately — only the level whose data is + // actually in flight shows a spinner. + const showLoading = + leafLoading || + (view.level === 'namespaces' && namespacesLoading) || + (view.level === 'domains' && domainsLoading); const isEmpty = !showLoading && rows.length === 0; return ( @@ -314,7 +328,7 @@ export function MobileNavMenu({ namespaceCounts, domainCounts, countsLoading, on
    {showLoading && (
  • - +
    • )} {isEmpty && ( From ae07e8c2569c18ff3dae2caa1ace172ef4e25f59 Mon Sep 17 00:00:00 2001 From: Shivaji Byrapaneni Date: Tue, 8 Sep 2026 11:51:55 +0100 Subject: [PATCH 3/6] fix(calm-hub-ui): address second review round on loading-state/sparkline slice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the 3 findings from a second review pass on this slice's PR: - ExploreRail/MobileNavMenu now render a distinct 'could not load' message when a counts fetch fails, instead of the same 'Nothing here' text used for a genuinely empty list. Threads Hub's existing namespaceCountsFailed through both surfaces, and adds the equivalent domainCountsFailed (which didn't exist before) for symmetry. - Sparkline's #2728 label-clipping fix only anchored the literal first and last dot; with enough versions in a narrow track, an interior dot close to the edge (e.g. the 2nd of many) could still overflow. Replaced the per-index special case with a CSS clamp() applied to every dot, so no label can leave the track regardless of dot count or track width — the true first/last still keep their edge-hugging text alignment, since they always hit the clamp's bound. - Added link role/href coverage for SectionHeader's namespace/type links (including a case with reserved URL characters), which had gained real navigation behaviour with no assertions on it. Claude-Session: https://claude.ai/code/session_0199XmacMNTrxWL4x5CSXyp1 --- calm-hub-ui/src/hub/Hub.tsx | 11 +- .../diagram-section/timeline/Sparkline.tsx | 126 +++++++++--------- .../timeline/TimelineBar.test.tsx | 27 +++- .../explore-rail/ExploreRail.test.tsx | 13 ++ .../components/explore-rail/ExploreRail.tsx | 10 ++ .../section-header/SectionHeader.test.tsx | 44 ++++++ .../tree-navigation/MobileNavMenu.test.tsx | 23 ++++ .../tree-navigation/MobileNavMenu.tsx | 13 +- 8 files changed, 199 insertions(+), 68 deletions(-) diff --git a/calm-hub-ui/src/hub/Hub.tsx b/calm-hub-ui/src/hub/Hub.tsx index e3ad4cbb36..5b4a7b8e8f 100644 --- a/calm-hub-ui/src/hub/Hub.tsx +++ b/calm-hub-ui/src/hub/Hub.tsx @@ -69,6 +69,8 @@ export default function Hub() { const [namespaceCountsFailed, setNamespaceCountsFailed] = useState(false); const [domainCounts, setDomainCounts] = useState([]); const [domainCountsLoaded, setDomainCountsLoaded] = useState(false); + // Mirrors namespaceCountsFailed above — a failed fetch means "unknown", not "zero". + const [domainCountsFailed, setDomainCountsFailed] = useState(false); const isMobile = useIsMobile(); // Route-first content selection (redesign problem #4): the same element @@ -112,7 +114,10 @@ export default function Hub() { countsService .fetchDomainCounts() .then(setDomainCounts) - .catch(() => setDomainCounts([])) + .catch(() => { + setDomainCounts([]); + setDomainCountsFailed(true); + }) .finally(() => setDomainCountsLoaded(true)); }, [countsService]); @@ -467,6 +472,8 @@ export default function Hub() { domainCounts={domainCounts} namespacesLoading={!namespaceCountsLoaded} domainsLoading={!domainCountsLoaded} + namespacesFailed={namespaceCountsFailed} + domainsFailed={domainCountsFailed} onCollapse={() => setIsSidebarOpen(false)} /> ) : ( @@ -502,6 +509,8 @@ export default function Hub() { domainCounts={domainCounts} namespacesLoading={!namespaceCountsLoaded} domainsLoading={!domainCountsLoaded} + namespacesFailed={namespaceCountsFailed} + domainsFailed={domainCountsFailed} onClose={() => setIsMobileNavOpen(false)} />
diff --git a/calm-hub-ui/src/hub/components/diagram-section/timeline/Sparkline.tsx b/calm-hub-ui/src/hub/components/diagram-section/timeline/Sparkline.tsx index 4c44b650a0..1f5c114ac7 100644 --- a/calm-hub-ui/src/hub/components/diagram-section/timeline/Sparkline.tsx +++ b/calm-hub-ui/src/hub/components/diagram-section/timeline/Sparkline.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from 'react'; +import { Fragment, useEffect, useRef, useState } from 'react'; import { IoChevronUpOutline } from 'react-icons/io5'; import { colors } from '../../../../theme/colors.js'; import { TimelineHeader } from './TimelineHeader.js'; @@ -24,7 +24,7 @@ interface ContextMenuState { y: number; } -/** Label box width; edge labels are anchored so this can never leave the card (#2728). */ +/** Label box width; labels are clamped to the track so this can never leave the card (#2728). */ const LABEL_WIDTH = 120; /** @@ -65,11 +65,7 @@ export function Sparkline({ // Progress overlay: left edge to the viewed dot in single mode; between // FROM and TO in compare mode. const progressLeft = comparing && fromIdx >= 0 && toIdx >= 0 ? pct(Math.min(fromIdx, toIdx)) : 0; - const progressRight = comparing && fromIdx >= 0 && toIdx >= 0 - ? pct(Math.max(fromIdx, toIdx)) - : viewedIdx >= 0 - ? pct(viewedIdx) - : 0; + const progressRight = comparing && fromIdx >= 0 && toIdx >= 0 ? pct(Math.max(fromIdx, toIdx)) : viewedIdx >= 0 ? pct(viewedIdx) : 0; useEffect(() => { if (!menu) return; @@ -154,21 +150,14 @@ export function Sparkline({ {/* Track row. A single-version resource has nothing to scrub, so the track is suppressed (the version pill already states what's shown). - Labels are bounded by LABEL_WIDTH + ellipsis, and the first/last are - anchored to their column so they stay inside DiagramSection's - overflow-hidden card rather than being sliced (#2728). */} + Labels are bounded by LABEL_WIDTH + ellipsis, and clamped to stay + inside the track — and so inside DiagramSection's overflow-hidden + card — for any dot, not just the true first/last (#2728). */} {!singleVersion && ( -
+
{/* Inner track wrapper inset 10px each side so dot percentages map directly */}
-
+
{progressRight > progressLeft && (
- +
+ +
+ {/* Positioned relative to the same inset wrapper as the dots (not + the 32px dot column) so clamp()'s percentages resolve against + the full track width — required for the clamp to be able to + keep the label inside the track for any dot, not just i===0/total-1. */} +
{moment.validFrom}
)}
-
+ ); })}
diff --git a/calm-hub-ui/src/hub/components/diagram-section/timeline/TimelineBar.test.tsx b/calm-hub-ui/src/hub/components/diagram-section/timeline/TimelineBar.test.tsx index efec3ffd2d..5f9c4d7dd5 100644 --- a/calm-hub-ui/src/hub/components/diagram-section/timeline/TimelineBar.test.tsx +++ b/calm-hub-ui/src/hub/components/diagram-section/timeline/TimelineBar.test.tsx @@ -257,7 +257,7 @@ describe('TimelineBar', () => { // #2728 — long moment names must not block the expand control or clip cards. describe('long moment names are bounded (#2728)', () => { - it('anchors the first and last collapsed labels to their column so they cannot leave the card', () => { + it('anchors the first and last collapsed labels so they cannot leave the card', () => { renderBar(); // The first/last dots sit close to the card edge, so a centred label // would overflow the ancestor card's overflow-hidden boundary and be @@ -267,6 +267,31 @@ describe('TimelineBar', () => { expect(screen.getByText('2.0.0')).toHaveStyle({ textAlign: 'right' }); }); + it('clamps every label to the track, not just the true first/last dot', () => { + // With enough versions, a *non-edge* dot (e.g. the 2nd of many) can sit + // close enough to the edge that centering its label would still overflow + // the card. A fixed set of 3 moments can't exercise this — the fix must + // hold for any dot count and track width, which a per-dot clamp() gives us + // (rather than only special-casing i===0/total-1). + const many: TimelineMoment[] = Array.from({ length: 12 }, (_, i) => ({ + key: `m${i}`, + label: `${i}.0.0`, + version: `${i}.0.0`, + })); + render( + + ); + // Dot 1 of 12 sits at 1/11 ≈ 9.09% along the track — close enough to the + // left edge that a centred 120px-wide label would still overflow. Its + // clamp() expression must reflect that dot's own position, not the + // static 50%-centered value the old per-index special case fell back to + // for every non-edge dot. + const secondLabel = screen.getByText('1.0.0'); + expect(secondLabel.parentElement).toHaveStyle({ + left: 'clamp(0px, calc(9.090909090909092% - 60px), calc(100% - 120px))', + }); + }); + it('truncates each collapsed label with an ellipsis while keeping its full-name tooltip', () => { renderBar(); const label = screen.getByText('1.0.0'); diff --git a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.test.tsx b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.test.tsx index 8c2910570f..3814517b98 100644 --- a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.test.tsx +++ b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.test.tsx @@ -18,6 +18,8 @@ interface RenderRailOptions { onCollapse?: () => void; namespacesLoading?: boolean; domainsLoading?: boolean; + namespacesFailed?: boolean; + domainsFailed?: boolean; } const renderRail = (path = '/', opts: RenderRailOptions = {}) => @@ -34,6 +36,8 @@ const renderRail = (path = '/', opts: RenderRailOptions = {}) => domainCounts={domainCounts} namespacesLoading={opts.namespacesLoading} domainsLoading={opts.domainsLoading} + namespacesFailed={opts.namespacesFailed} + domainsFailed={opts.domainsFailed} onCollapse={opts.onCollapse} /> } @@ -127,4 +131,13 @@ describe('ExploreRail', () => { expect(screen.getByRole('link', { name: /security/ })).toBeInTheDocument(); expect(screen.queryByRole('status')).not.toBeInTheDocument(); }); + + it('shows a distinct message when a counts fetch fails, rather than an ambiguous empty state', async () => { + renderRail('/', { namespacesLoading: false, domainsLoading: false, namespacesFailed: true, domainsFailed: true }); + // A failed fetch is "unknown", not "confirmed zero" — the empty-state text + // must say so rather than looking identical to a genuinely empty namespace. + expect(await screen.findByText("Couldn't load namespaces")).toBeInTheDocument(); + expect(screen.getByText("Couldn't load control domains")).toBeInTheDocument(); + expect(screen.queryByRole('link', { name: /finos/ })).not.toBeInTheDocument(); + }); }); diff --git a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx index 18de1e6c2c..587eadac92 100644 --- a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx +++ b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx @@ -16,6 +16,10 @@ interface ExploreRailProps { namespacesLoading?: boolean; /** True while the domain control counts are still being fetched. */ domainsLoading?: boolean; + /** True if the namespace counts fetch failed — distinct from "loaded and empty". */ + namespacesFailed?: boolean; + /** True if the domain counts fetch failed — distinct from "loaded and empty". */ + domainsFailed?: boolean; /** Collapse the rail (keeps the existing sidebar collapse affordance). */ onCollapse?: () => void; } @@ -49,6 +53,8 @@ export function ExploreRail({ domainCounts, namespacesLoading, domainsLoading, + namespacesFailed, + domainsFailed, onCollapse, }: ExploreRailProps) { // `ns` comes from /namespace/:ns; on the detail route /:namespace/:type/:id/:version the @@ -106,6 +112,8 @@ export function ExploreRail({
{namespacesLoading ? ( + ) : namespacesFailed ? ( + Couldn't load namespaces ) : filteredNamespaces.length === 0 ? ( Nothing here ) : ( @@ -125,6 +133,8 @@ export function ExploreRail({
{domainsLoading ? ( + ) : domainsFailed ? ( + Couldn't load control domains ) : domainCounts.length === 0 ? ( Nothing here ) : ( diff --git a/calm-hub-ui/src/hub/components/section-header/SectionHeader.test.tsx b/calm-hub-ui/src/hub/components/section-header/SectionHeader.test.tsx index b4aba41ec1..aaaedd6181 100644 --- a/calm-hub-ui/src/hub/components/section-header/SectionHeader.test.tsx +++ b/calm-hub-ui/src/hub/components/section-header/SectionHeader.test.tsx @@ -53,6 +53,50 @@ describe('SectionHeader', () => { expect(heading).not.toHaveTextContent('42'); }); + it('links the namespace and type label to the correct namespace/filtered-type routes', () => { + render( + Icon} + namespace="my-namespace" + id="42" + version="1.0.0" + typeSegment="architectures" + typeLabel="Architecture" + /> + ); + + expect(screen.getByRole('link', { name: 'my-namespace' })).toHaveAttribute( + 'href', + '/namespace/my-namespace' + ); + expect(screen.getByRole('link', { name: 'Architecture' })).toHaveAttribute( + 'href', + '/namespace/my-namespace?type=architectures' + ); + }); + + it('encodes namespace and type segments containing reserved URL characters', () => { + render( + Icon} + namespace="my namespace" + id="42" + version="1.0.0" + typeSegment="building blocks" + typeLabel="Building Block" + /> + ); + + expect(screen.getByRole('link', { name: 'my namespace' })).toHaveAttribute( + 'href', + '/namespace/my%20namespace' + ); + expect(screen.getByRole('link', { name: 'Building Block' })).toHaveAttribute( + 'href', + '/namespace/my%20namespace?type=building%20blocks' + ); + }); + it('renders right content when provided', () => { const icon = Icon; const rightContent =
Right Content
; diff --git a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.test.tsx b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.test.tsx index 9c623dc400..fbaf86bc37 100644 --- a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.test.tsx +++ b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.test.tsx @@ -215,4 +215,27 @@ describe('MobileNavMenu', () => { expect(screen.getByRole('status')).toBeInTheDocument(); expect(screen.queryByText('security')).not.toBeInTheDocument(); }); + + it('shows a distinct message when a counts fetch fails, rather than an ambiguous empty state', async () => { + // Hub clears counts to [] on a failed fetch, so the failure looks + // identical to a genuinely empty namespace/domain list unless the + // *Failed flag is threaded through to distinguish "unknown" from "zero". + render( + + + + ); + fireEvent.click(screen.getByText('Namespaces')); + expect(await screen.findByText("Couldn't load — try again")).toBeInTheDocument(); + + fireEvent.click(screen.getByLabelText('Back')); + fireEvent.click(screen.getByText('Control Domains')); + expect(await screen.findByText("Couldn't load — try again")).toBeInTheDocument(); + }); }); diff --git a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx index 68c30a1820..4848851136 100644 --- a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx +++ b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx @@ -31,6 +31,10 @@ interface MobileNavMenuProps { namespacesLoading?: boolean; /** True while the domain control counts are still being fetched. */ domainsLoading?: boolean; + /** True if the namespace counts fetch failed — distinct from "loaded and empty". */ + namespacesFailed?: boolean; + /** True if the domain counts fetch failed — distinct from "loaded and empty". */ + domainsFailed?: boolean; /** Dismiss the menu (e.g. after a resource is chosen). */ onClose: () => void; } @@ -75,6 +79,8 @@ export function MobileNavMenu({ domainCounts, namespacesLoading, domainsLoading, + namespacesFailed, + domainsFailed, onClose, }: MobileNavMenuProps) { const navigate = useNavigate(); @@ -305,6 +311,11 @@ export function MobileNavMenu({ (view.level === 'namespaces' && namespacesLoading) || (view.level === 'domains' && domainsLoading); const isEmpty = !showLoading && rows.length === 0; + // Distinguish "the fetch failed" from "there's genuinely nothing here" — a + // failed counts fetch is unknown, not zero (mirrors Hub's own namespaceCountsFailed). + const failed = + (view.level === 'namespaces' && namespacesFailed) || (view.level === 'domains' && domainsFailed); + const emptyMessage = failed ? "Couldn't load — try again" : 'Nothing here'; return (
@@ -332,7 +343,7 @@ export function MobileNavMenu({ )} {isEmpty && ( -
  • Nothing here
    • +
  • {emptyMessage}
    • )} {!showLoading && rows.map((row) => ( From 84827d3de640a9e1b50a6241547a02fdf4e6b2ce Mon Sep 17 00:00:00 2001 From: Shivaji Byrapaneni Date: Tue, 8 Sep 2026 12:03:44 +0100 Subject: [PATCH 4/6] =?UTF-8?q?fix(calm-hub-ui):=20address=20third=20revie?= =?UTF-8?q?w=20round=20=E2=80=94=20domain-count=20failure=20gap=20and=20sp?= =?UTF-8?q?inner=20duplication?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - domainControlCount/controlDomainCount in Hub.tsx only gated on domainCountsLoaded, not the domainCountsFailed added in the previous round, so a failed domain-counts fetch rendered a confirmed '0 controls' instead of unknown — inconsistent with the comment right above claiming it mirrors activeNamespaceCounts, which does check the failed flag. Both now gate on both flags; added a Hub-level test simulating the failure via a spy on the mocked CountsService. - Extracted the identical spinner span duplicated across ExploreRail and MobileNavMenu (both touched by this same PR) into a shared LoadingSpinner, so a future style change can't drift between them. Each component keeps its own wrapper/spacing, which differ by context (rail div vs mobile list item). Claude-Session: https://claude.ai/code/session_0199XmacMNTrxWL4x5CSXyp1 --- calm-hub-ui/src/hub/Hub.test.tsx | 9 +++++++++ calm-hub-ui/src/hub/Hub.tsx | 20 ++++++++++++------- .../src/hub/components/LoadingSpinner.tsx | 9 +++++++++ .../components/explore-rail/ExploreRail.tsx | 3 ++- .../tree-navigation/MobileNavMenu.tsx | 3 ++- 5 files changed, 35 insertions(+), 9 deletions(-) create mode 100644 calm-hub-ui/src/hub/components/LoadingSpinner.tsx diff --git a/calm-hub-ui/src/hub/Hub.test.tsx b/calm-hub-ui/src/hub/Hub.test.tsx index 9dc6215a5b..2f7ed7d3f0 100644 --- a/calm-hub-ui/src/hub/Hub.test.tsx +++ b/calm-hub-ui/src/hub/Hub.test.tsx @@ -4,6 +4,7 @@ import { MemoryRouter, useLocation, useNavigate } from 'react-router-dom'; import Hub from './Hub.js'; import { vi, describe, it, expect, afterEach, beforeEach } from 'vitest'; import { authStore } from '../service/utils/auth-store.js'; +import { CountsService } from '../service/counts-service.js'; import type { Data, Adr } from '../model/calm.js'; import type { ControlData } from '../model/control.js'; import type { InterfaceData } from '../model/interface.js'; @@ -475,6 +476,14 @@ describe('Hub', () => { expect(await screen.findByTestId('domain-page')).toHaveTextContent('Domain: security (3)'); }); + it('passes an unknown (undefined) control count, not a misleading 0, when the domain counts fetch fails', async () => { + // A failed fetch means the count is unknown, not a confirmed zero — + // the same distinction Hub already makes for namespace counts. + vi.spyOn(CountsService.prototype, 'fetchDomainCounts').mockRejectedValueOnce(new Error('boom')); + renderAt('/domain/security'); + expect(await screen.findByTestId('domain-page')).toHaveTextContent('Domain: security ()'); + }); + it('renders the intro (not a namespace/domain page) on the empty / route', async () => { renderAt('/'); expect(screen.queryByTestId('namespace-page')).not.toBeInTheDocument(); diff --git a/calm-hub-ui/src/hub/Hub.tsx b/calm-hub-ui/src/hub/Hub.tsx index 5b4a7b8e8f..5e9e5851fb 100644 --- a/calm-hub-ui/src/hub/Hub.tsx +++ b/calm-hub-ui/src/hub/Hub.tsx @@ -345,20 +345,26 @@ export default function Hub() { } ); }, [namespaceCounts, namespaceCountsLoaded, namespaceCountsFailed, activeNamespace]); - // Both counts stay `undefined` until the domain-counts fetch settles, so a - // deep-link shows "controls" rather than a misleading "0 controls" before it - // resolves (mirrors the activeNamespaceCounts gate above). + // Both counts stay `undefined` until the domain-counts fetch settles OR if it + // failed, so a deep-link shows "controls" rather than a misleading "0 controls" + // (mirrors the activeNamespaceCounts gate above). const domainControlCount = useMemo( - () => (domainCountsLoaded ? (domainCounts.find((c) => c.domain === activeDomain)?.controlCount ?? 0) : undefined), - [domainCounts, domainCountsLoaded, activeDomain] + () => + !domainCountsLoaded || domainCountsFailed + ? undefined + : (domainCounts.find((c) => c.domain === activeDomain)?.controlCount ?? 0), + [domainCounts, domainCountsLoaded, domainCountsFailed, activeDomain] ); // Count for the grid shown behind a selected control's panel — the control's own // domain, which may differ from the route's activeDomain when reached via the // detail route (deep-link / mobile drill-down). const controlDomain = controlData?.domain; const controlDomainCount = useMemo( - () => (domainCountsLoaded ? (domainCounts.find((c) => c.domain === controlDomain)?.controlCount ?? 0) : undefined), - [domainCounts, domainCountsLoaded, controlDomain] + () => + !domainCountsLoaded || domainCountsFailed + ? undefined + : (domainCounts.find((c) => c.domain === controlDomain)?.controlCount ?? 0), + [domainCounts, domainCountsLoaded, domainCountsFailed, controlDomain] ); // Chrome-free intro / front door (`/` with nothing else active): early-returns diff --git a/calm-hub-ui/src/hub/components/LoadingSpinner.tsx b/calm-hub-ui/src/hub/components/LoadingSpinner.tsx new file mode 100644 index 0000000000..ca15e6e21f --- /dev/null +++ b/calm-hub-ui/src/hub/components/LoadingSpinner.tsx @@ -0,0 +1,9 @@ +/** + * Shared spinner markup for the drill-down/browse navigation surfaces + * (ExploreRail, MobileNavMenu). Callers own their own wrapper element and + * spacing (a rail `
    ` vs a mobile `
  • `), since those differ by context — + * only the spinner itself needs to stay identical between them. + */ +export function LoadingSpinner({ label }: { label: string }) { + return ; +} diff --git a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx index 587eadac92..73fd3e49f0 100644 --- a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx +++ b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx @@ -6,6 +6,7 @@ import { colors } from '../../../theme/colors.js'; import { redesignTokens } from '../../../theme/redesign-tokens.js'; import { RailItem } from './RailItem.js'; import { RailSectionLabel } from './RailSectionLabel.js'; +import { LoadingSpinner } from '../LoadingSpinner.js'; interface ExploreRailProps { /** Per-namespace counts, fetched once by {@link Hub} and passed down. */ @@ -27,7 +28,7 @@ interface ExploreRailProps { function RailSpinner({ label }: { label: string }) { return (
    - +
    ); } diff --git a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx index 4848851136..b55b7ec9c0 100644 --- a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx +++ b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx @@ -19,6 +19,7 @@ import { fetchVersionsForResource, } from './navigation-loaders.js'; import { ExplorerSearch } from '../../../components/navbar/ExplorerSearch.js'; +import { LoadingSpinner } from '../LoadingSpinner.js'; const RESOURCE_TYPES: TypeInUI[] = ['Architectures', 'Patterns', 'Flows', 'Standards', 'ADRs', 'Interfaces']; @@ -339,7 +340,7 @@ export function MobileNavMenu({
      {showLoading && (
    • - +
      • )} {isEmpty && ( From 117452c58b0f97df283b65189177ca335426f753 Mon Sep 17 00:00:00 2001 From: Shivaji Byrapaneni Date: Tue, 8 Sep 2026 12:18:30 +0100 Subject: [PATCH 5/6] =?UTF-8?q?fix(calm-hub-ui):=20address=20fourth=20revi?= =?UTF-8?q?ew=20round=20=E2=80=94=20misleading=20copy=20and=20ambiguous=20?= =?UTF-8?q?filter-empty=20state?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - MobileNavMenu's failure message said 'try again' but there's no retry path (Hub fetches counts once on mount) — replaced with level-specific wording matching ExploreRail's equivalent copy. - ExploreRail's 'Nothing here' fired identically whether the filter matched nothing or there were genuinely no namespaces; the two are now distinguished. Not addressed, noted for the record: - The same unknown-vs-zero distinction doesn't reach MobileNavMenu's leaf-level fetches (resource types, controls) — those catch blocks predate this PR and are unrelated to the counts-loading fix this slice makes; flagged as a separate follow-up on the PR. - Sparkline's label clamp() inverts if the track ever drops under 120px; not reachable at any current desktop or mobile viewport in this codebase per review verification, so left as a known theoretical edge rather than adding a speculative width guard. Claude-Session: https://claude.ai/code/session_0199XmacMNTrxWL4x5CSXyp1 --- .../hub/components/explore-rail/ExploreRail.test.tsx | 8 ++++++++ .../src/hub/components/explore-rail/ExploreRail.tsx | 4 +++- .../components/tree-navigation/MobileNavMenu.test.tsx | 5 +++-- .../hub/components/tree-navigation/MobileNavMenu.tsx | 11 ++++++++--- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.test.tsx b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.test.tsx index 3814517b98..85d8a29439 100644 --- a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.test.tsx +++ b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.test.tsx @@ -83,6 +83,14 @@ describe('ExploreRail', () => { expect(screen.getByRole('link', { name: /security/ })).toBeInTheDocument(); }); + it('tells a filter matching nothing apart from a genuinely empty namespace list', async () => { + renderRail(); + await screen.findByRole('link', { name: /finos/ }); + + fireEvent.change(screen.getByLabelText('Filter namespaces'), { target: { value: 'no-such-namespace' } }); + expect(screen.getByText('No namespaces match your filter')).toBeInTheDocument(); + }); + it('marks the namespace row matching the URL as active', async () => { renderRail('/namespace/traderx'); const active = await screen.findByRole('link', { name: /traderx/ }); diff --git a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx index 73fd3e49f0..75d029aa12 100644 --- a/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx +++ b/calm-hub-ui/src/hub/components/explore-rail/ExploreRail.tsx @@ -116,7 +116,9 @@ export function ExploreRail({ ) : namespacesFailed ? ( Couldn't load namespaces ) : filteredNamespaces.length === 0 ? ( - Nothing here + + {namespaceCounts.length === 0 ? 'Nothing here' : 'No namespaces match your filter'} + ) : ( filteredNamespaces.map((nc) => ( { ); fireEvent.click(screen.getByText('Namespaces')); - expect(await screen.findByText("Couldn't load — try again")).toBeInTheDocument(); + // No retry action exists here, so the copy must not promise one. + expect(await screen.findByText("Couldn't load namespaces")).toBeInTheDocument(); fireEvent.click(screen.getByLabelText('Back')); fireEvent.click(screen.getByText('Control Domains')); - expect(await screen.findByText("Couldn't load — try again")).toBeInTheDocument(); + expect(await screen.findByText("Couldn't load control domains")).toBeInTheDocument(); }); }); diff --git a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx index b55b7ec9c0..22a4eb846d 100644 --- a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx +++ b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx @@ -314,9 +314,14 @@ export function MobileNavMenu({ const isEmpty = !showLoading && rows.length === 0; // Distinguish "the fetch failed" from "there's genuinely nothing here" — a // failed counts fetch is unknown, not zero (mirrors Hub's own namespaceCountsFailed). - const failed = - (view.level === 'namespaces' && namespacesFailed) || (view.level === 'domains' && domainsFailed); - const emptyMessage = failed ? "Couldn't load — try again" : 'Nothing here'; + // No retry action exists here (Hub fetches counts once on mount), so the copy + // must not promise one — matches ExploreRail's equivalent desktop wording. + const emptyMessage = + view.level === 'namespaces' && namespacesFailed + ? "Couldn't load namespaces" + : view.level === 'domains' && domainsFailed + ? "Couldn't load control domains" + : 'Nothing here'; return (
      From 1dba0778106181f12d21faa67126b049264c513c Mon Sep 17 00:00:00 2001 From: Shivaji Byrapaneni Date: Tue, 8 Sep 2026 12:27:36 +0100 Subject: [PATCH 6/6] =?UTF-8?q?fix(calm-hub-ui):=20address=20fifth=20revie?= =?UTF-8?q?w=20round=20=E2=80=94=20sparkline=20vertical=20overflow=20and?= =?UTF-8?q?=20generic=20spinner=20label?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Removing the sparkline track's overflow:hidden (for the #2728 clamp() fix) also dropped vertical containment on the validFrom date row, which had no whiteSpace/overflow bound of its own — an unusually long value could now bleed past the track instead of being clipped. Gave it the same single-line ellipsis treatment the label row already has. - MobileNavMenu's drill-down spinner used a bare 'Loading' label regardless of which section was loading, unlike ExploreRail's section-specific labels on the desktop equivalent. Now says 'Loading namespaces'/'Loading control domains' to match. Declined as architecture/DRY preferences rather than defects (noted on the PR): generalizing the namespace/domain loading-failed state into a shared async-resource abstraction, and deduplicating the loading/failed/empty branch between ExploreRail and MobileNavMenu. Both are legitimate refactors but out of scope for a review-fix pass — the current duplication is small (a few lines) and each component's render differs enough (div vs li, different spacing) that a shared abstraction would add its own indirection cost. Claude-Session: https://claude.ai/code/session_0199XmacMNTrxWL4x5CSXyp1 --- .../diagram-section/timeline/Sparkline.tsx | 14 +++++++++++++- .../tree-navigation/MobileNavMenu.test.tsx | 4 +++- .../components/tree-navigation/MobileNavMenu.tsx | 6 +++++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/calm-hub-ui/src/hub/components/diagram-section/timeline/Sparkline.tsx b/calm-hub-ui/src/hub/components/diagram-section/timeline/Sparkline.tsx index 1f5c114ac7..150f72ba16 100644 --- a/calm-hub-ui/src/hub/components/diagram-section/timeline/Sparkline.tsx +++ b/calm-hub-ui/src/hub/components/diagram-section/timeline/Sparkline.tsx @@ -256,7 +256,19 @@ export function Sparkline({ {moment.validFrom && (
      {moment.validFrom}
      diff --git a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.test.tsx b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.test.tsx index 36ffe417cc..6b3635c9d2 100644 --- a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.test.tsx +++ b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.test.tsx @@ -212,7 +212,9 @@ describe('MobileNavMenu', () => { fireEvent.click(screen.getByLabelText('Back')); fireEvent.click(screen.getByText('Control Domains')); - expect(screen.getByRole('status')).toBeInTheDocument(); + // Section-specific, matching ExploreRail's equivalent spinner labels — + // not a bare "Loading" that doesn't say which section to a screen reader. + expect(screen.getByRole('status', { name: 'Loading control domains' })).toBeInTheDocument(); expect(screen.queryByText('security')).not.toBeInTheDocument(); }); diff --git a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx index 22a4eb846d..6e47362d29 100644 --- a/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx +++ b/calm-hub-ui/src/hub/components/tree-navigation/MobileNavMenu.tsx @@ -311,6 +311,10 @@ export function MobileNavMenu({ leafLoading || (view.level === 'namespaces' && namespacesLoading) || (view.level === 'domains' && domainsLoading); + // Matches ExploreRail's section-specific spinner labels, rather than a bare + // "Loading" that doesn't tell a screen-reader user which section. + const loadingLabel = + view.level === 'namespaces' ? 'Loading namespaces' : view.level === 'domains' ? 'Loading control domains' : 'Loading'; const isEmpty = !showLoading && rows.length === 0; // Distinguish "the fetch failed" from "there's genuinely nothing here" — a // failed counts fetch is unknown, not zero (mirrors Hub's own namespaceCountsFailed). @@ -345,7 +349,7 @@ export function MobileNavMenu({
        {showLoading && (
      • - +
        • )} {isEmpty && (