Conversation
A namespace has a dotted name, so the hierarchy is already in the name. The rail did not use it. It drew finos.calm and finos.traderx as siblings of finos rather than below it, which does not scale to an enterprise with hundreds of namespaces. The rows now nest on the dot prefix. A row has two hit targets. The chevron opens and closes the subtree. The label opens the page of that namespace. A prefix that no namespace claims draws as a group-only row, muted and not clickable. A filter flattens the tree to the matches at any depth and shows full names, so finos.traderx stays distinct from traderx. The trie builder is recovered from TreeNavigation.tsx, which commit 6104751 deleted. Its path compression is dropped, so each segment gets its own row. Part of finos#2787. The fly-out for the closed rail follows in a second PR.
96ab4fd to
bfed4bb
Compare
Collapsing the rail unmounted it and left a 48px strip holding only an expand chevron. The rail now keeps its content: one initial per root namespace, accented when the active namespace sits under it, and a fly-out of that subtree on hover or focus. The fly-out is transient, so it carries no collapse state and no chevrons. It flattens the subtree in full. The collapsed strip now matches the expanded rail — same surface, same right border, flush to the edge — rather than floating as a rounded card inside grey padding. Part of finos#2787.
The mobile drawer showed namespaces as a flat list, so the hierarchy was visible on desktop only. The namespaces level now renders the same tree. A row has a 40px chevron column and a label, each at least 44px tall. The chevron opens and closes children in place and the drawer stays open. Expansion persists through the same stored set the desktop rail uses. The label keeps its present behaviour and opens that namespace's resource types. finos#2787 asks it to navigate and dismiss the drawer, but that tap is the only route into the types and resources levels, so following it would leave both unreachable and remove mobile's only path from the drawer to a specific architecture. calm-hub-ui/AGENTS.md said the mobile explorer is one flat list per level and never a tree. That rule predates this issue. It now covers the types, resources, domains and controls levels, and names the namespaces level as the exception. Closes finos#2787.
…il' into feat/2787-namespace-hierarchy-rail
aamanrebello
left a comment
aamanrebello
left a comment
There was a problem hiding this comment.
This review was run with AI-assisted tooling (Claude Code) against this branch's diff. Sorry for the comment volume from an AI pass — please feel free to respond on any comment you think does not apply, or where the tradeoff was intentional.
| ))} | ||
| </div> | ||
|
|
||
| {hasChildren ? ( |
There was a problem hiding this comment.
The chevron button renders whenever hasChildren is true, without checking filtering. In filter mode, flattenNamespaceTree always returns collapsed: false and ignores the collapsed set, so clicking the chevron here has no visible effect — but it still calls onToggleCollapsed, which flips the entry in the persisted collapse set. After the user clears the filter, a namespace can appear collapsed or expanded that they never consciously toggled. Consider disabling the toggle (or hiding the button) while filtering is true.
| } from './namespace-tree.js'; | ||
|
|
||
| /** Persist which namespaces are collapsed so a refresh keeps the rail as it was. */ | ||
| const COLLAPSED_STORAGE_KEY = 'calmHub.railCollapsedNamespaces'; |
There was a problem hiding this comment.
COLLAPSED_STORAGE_KEY is one global key, but this hook is used by both ExploreRail (desktop) and MobileNavMenu (mobile), both defaulting storage to localStorage. Collapsing a namespace on one surface silently changes the starting state on the other, even though they show different node sets. Consider namespacing the key per surface (e.g. a storageKey param).
| if (!isWithin(e.currentTarget, e.relatedTarget)) onClose(); | ||
| }} | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Escape') onClose(); |
There was a problem hiding this comment.
Escape closes the fly-out unconditionally, which unmounts the {isOpen && (...)} block. If a keyboard user has tabbed onto one of the fly-out's Link rows, that element disappears and the browser drops focus to <body> — nothing here returns focus to the trigger button. Worth keeping a ref to the trigger and calling .focus() on it after closing (the same pattern onBlur already uses for detecting focus movement).
| <div | ||
| className="relative" | ||
| onMouseEnter={onOpen} | ||
| onMouseLeave={onClose} |
There was a problem hiding this comment.
onMouseLeave closes the fly-out unconditionally, unlike onBlur just below it, which checks isWithin before closing. A hybrid mouse+keyboard user who opens the fly-out via Tab (onFocus) and then nudges the mouse across/off the container will have it closed even though focus is still logically inside. Might be worth mirroring the isWithin check here, or dropping onMouseLeave in favor of onBlur.
| <div | ||
| className="absolute top-0 left-full ml-1 flex flex-col gap-0.5 p-1.5 rounded-[12px] z-50" | ||
| style={{ | ||
| width: 220, |
There was a problem hiding this comment.
This panel has a fixed width: 220 and no maxHeight/overflowY, and rows is built with collapsed: new Set() — so it always renders the full subtree regardless of what the user collapsed on the main rail. For a root with a large or deep subtree this can overflow the viewport with no way to scroll. A capped maxHeight plus overflowY: 'auto' would be a cheap safety net.
| export function buildNamespaceTree(namespaceCounts: NamespaceCounts[]): NamespaceTreeNode[] { | ||
| const root: TrieNode = { total: null, children: new Map() }; | ||
| for (const nc of namespaceCounts) { | ||
| const segments = (nc.namespace ?? '').split('.').filter(Boolean); |
There was a problem hiding this comment.
.filter(Boolean) drops empty path segments, so e.g. "billing..invoices" normalizes to the same trie path as a legitimate "billing.invoices". If both exist, whichever is processed second silently overwrites current.total for the first, and the rendered row links via the normalized path, not the original string. Not sure if malformed namespace strings like this can actually reach this component — if they can't (e.g. validated upstream), feel free to disregard.
| {namespaceCounts.length === 0 ? 'Nothing here' : 'No namespaces match your filter'} | ||
| </RailEmpty> | ||
| ) : rows.length === 0 ? ( | ||
| <RailEmpty>{needle === '' ? 'Nothing here' : 'No namespaces match your filter'}</RailEmpty> |
There was a problem hiding this comment.
This used to check namespaceCounts.length === 0 for the "nothing fetched" message; now it checks needle === ''. These aren't equivalent — if namespaceCounts is non-empty but buildNamespaceTree/flattenNamespaceTree ends up producing zero rows for some other reason (e.g. the malformed-namespace case noted on namespace-tree.ts:37), this now reports "Nothing here" as if the fetch failed, even though it didn't. Might be worth deriving this from whether the tree actually produced rows rather than from the filter text.
| className="font-mono-jb text-[11px] leading-none px-1.5 py-0.5 rounded-md min-w-[20px] text-center inline-block" | ||
| style={{ backgroundColor: colors.redesign.badgeBgFaint, color: colors.redesign.disabled }} | ||
| > | ||
| +{count} |
There was a problem hiding this comment.
No guard for count === 0 — if a collapsed namespace's descendants are all zero-total, this renders a "+0" pill, implying hidden content that isn't there. A descendantTotal > 0 check at the call site (or a guard here) would avoid it.
| path: string; | ||
| /** Last path segment — what a row shows outside filter mode. */ | ||
| segment: string; | ||
| total: number | null; |
There was a problem hiding this comment.
node.total !== null (the "group-only vs. real namespace" check) is independently re-derived in NamespaceRailItem.tsx:55, CollapsedRail.tsx:20, and MobileNamespaceRow.tsx:25. Not a bug today, but if this sentinel ever changes, a fix applied to one and missed in another would silently make a namespace clickable in some views and not others. Might be worth an isNamespace(node) helper exported from here — could easily be a follow-up rather than part of this PR.
| const isNamespace = node.total !== null; | ||
|
|
||
| return ( | ||
| <div className="flex items-stretch" style={{ paddingLeft: depth * INDENT_PER_DEPTH }}> |
There was a problem hiding this comment.
Indentation-per-depth is implemented three different, uncoordinated ways: NamespaceRailItem.tsx caps at MAX_INDENT_GUIDES = 4 (14px each), CollapsedRail.tsx's FlyoutRow uses 8 + depth * 14 uncapped, and this uses depth * 16 uncapped. A namespace nested 10 levels deep would push 160px of padding on a mobile row, with no cap like the desktop rail has. Depends on how deep namespaces realistically nest in practice — if it's rare this might not matter, but a shared cap (mirroring the desktop pattern) would be cheap insurance.
This branch has not been deployed
2 participants
Description
A CALM Hub namespace has a dotted name. The hierarchy is therefore already in the name. The Explore rail did not use it. It drew
finos.calmandfinos.traderxas siblings offinos, not below it. This is easy to read at demo scale. It does not scale to an enterprise with hundreds of namespaces at several levels. @markscott-ms raised this point on the redesign epic #2754.The rail now nests rows on the dot prefix, and the filter flattens the tree to the matches. Collapsing the rail keeps that structure as a strip of root initials with a fly-out. The mobile drawer shows the same tree.
Closes #2787.
Visual Sample
Nested View
Filtered Search
Collapsed View with pop out. Namespaces with same first character appear duplicated however the pop out provides a form of disambiguation
Mobile ViewDecisions worth knowing
A row has two hit targets. The chevron opens and closes the subtree. The label opens the page of that namespace. A prefix such as
finosowns artefacts and also parents other namespaces, so a user must be able to do each action separately. This is why a namespace row is a new component. The existingRailItemis a single link, and it stays untouched for theCONTROL DOMAINSrows.Two counts, two weights. The solid pill is the own count of the row, unchanged from today. The ghost
+Npill is the sum the descendants hold, and a row shows it only while closed. The two add.finosowning 10 with children of 5 and 3 reads+8and[10], so the branch holds 18.The rail stores the closed rows, not the open ones. Every row is open by default, so each namespace visible before this change is still visible after it. Storing the open rows would show only roots on a first load, which reads as lost data.
Filtering never writes to that stored set. A filter opens ancestors to reveal a match, but it does so in the render only. Clearing the filter restores exactly what the user chose.
No
role="tree". That ARIA pattern wants one focusable element per row under a roving tabindex, which is incompatible with the two hit targets above. The rows are disclosure buttons and links in the natural tab order instead. This codebase has norole="tree"anywhere.Reuse
The trie builder is recovered from
TreeNavigation.tsx, which commit61047513deleted when the flat rail replaced the old tree, along with its unit tests. Two changes are deliberate: it no longer compresses paths, so each segment gets a row, and it now sorts siblings rather than relying on insertion order.The client tree agrees with the rule the server already uses to decide what a child namespace is.
Gotchas
The namespace href now runs through
encodeURIComponent. That changes nothing for any name the server permits. It is protection against a malformed payload, not a correction of a fault.The rail is 236px wide. A filter shows full paths so
finos.traderxstays distinct fromtraderx, and a match four levels deep will still truncate.The collapsed strip previously floated as a rounded card inside grey padding, while the expanded rail sits flush. It now matches the expanded rail, so collapsing narrows the rail rather than swapping one shape for another.
Mobile, and a convention this changes
Mobile does not render this rail.
Hub.tsxdraws a separate drill-down component,MobileNavMenu, where each tap replaces the list with the next flat level. Its namespaces level now renders the same tree, with a 40px chevron column and a label as separate targets, each at least 44px tall. A closed row shows the same ghost+Npill the desktop rail does. Expansion persists through the same stored set the desktop rail uses.One deliberate departure from the issue. #2787 says the mobile label should navigate to the namespace page and dismiss the drawer. That tap is the only route into the types and resources levels, so following it would leave both unreachable and remove mobile's only path from the drawer to a specific architecture. The label therefore keeps its present behaviour and opens the types list. The chevron carries the new expand action.
Mobile has no namespace filter, deliberately. The drawer already carries one text input,
ExplorerSearch, which queries the hub and replaces the list rather than narrowing it. A second input filtering the list beneath it would put two text fields with different jobs at the top of a 390px drawer. Drilling into a branch is the mobile equivalent of filtering, so the tree is built with filtering switched off there.That leaves one asymmetry worth knowing. Both surfaces share a single stored set of closed rows, so a branch closed on the desktop rail is closed in the drawer too. On the desktop a filter reveals it regardless, because filtering ignores that set. On mobile the chevron is the only way back. The default is fully open, so this reaches only a user who closed the branch on purpose.
This changes a documented convention, so it needs a reviewer's eye.
calm-hub-ui/AGENTS.mdsaid the mobile explorer is one flat list per level and never a tree. That rule predates #2787 by sixteen days, and #2787 asks for a tree in the component the rule names. The rule is now narrowed rather than dropped: it still governs the types, resources, domains and controls levels, and names the namespaces level as the exception.Type of Change
The API does not change. The data model does not change. The hierarchy is a pure function of names the counts endpoint already returns.
Affected Components
cli/)calm/)calm-ai/)calm-hub/)calm-hub-ui/)calm-server/)calm-widgets/)docs/)shared/)calm-plugins/vscode/)Commit Message Format ✅
feat(calm-hub-ui): nest namespaces in the Explore railfeat(calm-hub-ui): show namespace roots in the collapsed railTesting
This PR adds 44 tests. Every existing test passes without a change, including all twelve
ExploreRail.test.tsxcases and theHub.test.tsxexpectations for the collapse button. That was a constraint of the design, not a hoped-for result.One decision carries the most weight, so I checked it by mutation. If the flatten step reads the stored set of closed rows while a filter is active, exactly three tests fail — one in the pure module, one in the hook, one in the integration suite.
calm-hub-uipasses 135 files and 1610 tests. Lint reports 0 errors and 9 warnings, all pre-existing and none in a new file. The build is clean.Checked by hand against a standalone hub holding
finos,finos.calm,finos.traderx,traderxandtest-ns, in both the expanded and the collapsed state.On viewport coverage: this rail is not drawn on mobile at all, so there is no mobile render of it to test. The suite for the mobile drawer is untouched and passes.
Checklist
🤖 Generated with Claude Code