NMS-19878: Topology Preview UI - #8803
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new “Topology (Preview)” UI entry point and a substantial set of supporting topology UI utilities/components, along with new @opennms/onms-ui seam wrappers and contract tests to stabilize PrimeVue usage.
Changes:
- Introduces a new Topology preview route/menu entry and adjusts app layout behavior so the topology canvas can fill the viewport cell (preventing footer/layout overflow issues).
- Adds core topology UI building blocks (palette + drag payload, saved-view manager, location mini-map, graph source menu building, layout/search/severity/id/icon utilities).
- Expands the
@opennms/onms-uiseam (new wrappers like virtual scroller/tiered menu/context menu/select button/slider/color picker, plus additional mapped props) and adds/updates contract tests; adds sigma/graphology-related dependencies.
Reviewed changes
Copilot reviewed 75 out of 76 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/tests/onms-ui/OnmsVirtualScroller.test.ts | Contract tests for new virtual scroller wrapper (prop + slot contract). |
| ui/tests/onms-ui/OnmsTieredMenu.test.ts | Contract tests for new tiered menu wrapper. |
| ui/tests/onms-ui/OnmsTable.test.ts | Adds tests for selectionMode mapping and row-click forwarding. |
| ui/tests/onms-ui/OnmsSlider.test.ts | Contract tests for new slider wrapper. |
| ui/tests/onms-ui/OnmsSelectButton.test.ts | Contract tests for new select button wrapper and event/value behavior. |
| ui/tests/onms-ui/OnmsSearchInput.test.ts | Tests new fluid behavior for search input. |
| ui/tests/onms-ui/OnmsMultiSelect.test.ts | Tests new MultiSelect prop mappings. |
| ui/tests/onms-ui/OnmsInputNumber.test.ts | Tests new InputNumber spinner/button mappings. |
| ui/tests/onms-ui/OnmsContextMenu.test.ts | Contract tests for new context menu wrapper. |
| ui/tests/onms-ui/OnmsColorPicker.test.ts | Contract tests for new composite color picker. |
| ui/tests/onms-ui/OnmsButton.test.ts | Tests new button sizing behavior. |
| ui/tests/onms-ui/exports.test.ts | Updates runtime export expectations for new onms-ui components. |
| ui/tests/containers/AppLayoutClasses.test.ts | Tests route-scoped layout bounding/fill CSS classes. |
| ui/tests/components/Topology/ViewManager.test.ts | Tests toast wording and rename/delete flows for view manager dialog. |
| ui/tests/components/Topology/TopologyLocationMap.test.ts | Tests tile server config/theme behavior for mini-map. |
| ui/tests/components/Topology/TopologyExplorePanel.test.ts | Extensive tests for explore panel refresh/filtering/chunking behavior. |
| ui/tests/components/Topology/sources.test.ts | Tests derived/curated topology source menu building and variant logic. |
| ui/tests/components/Topology/severity.test.ts | Tests severity color mapping and cleared-alarm handling. |
| ui/tests/components/Topology/search.test.ts | Tests node/category search behavior and ranking. |
| ui/tests/components/Topology/nodeIds.test.ts | Tests canvas ID helpers. |
| ui/tests/components/Topology/nodeActions.test.ts | Tests cross-link URL generation for node actions. |
| ui/tests/components/Topology/linkHints.test.ts | Tests ghost link hint computation and binding resolution. |
| ui/tests/components/Topology/layout.test.ts | Tests discovered + hierarchy layouts and edge curvature logic. |
| ui/tests/components/Topology/interfaceState.test.ts | Tests IF-MIB state naming and provenance/age formatting. |
| ui/tests/components/Topology/focus.test.ts | Tests focus-subgraph selection and default focus selection logic. |
| ui/tests/components/Topology/deviceIcons.test.ts | Tests sysObjectId/iconKey mapping and VMware power-state badge behavior. |
| ui/src/types/topology.ts | Adds topology domain types (views, discovered graphs, bindings, annotations, etc.). |
| ui/src/theme/opennms-preset.ts | Theme override to fix SelectButton/ToggleButton checked text color in dark mode. |
| ui/src/styles/onms-theme.scss | Adds topology-specific CSS tokens for accent/link coloring (light + dark). |
| ui/src/services/geolocationService.ts | Caches geolocation config request per install to avoid repeated fetches. |
| ui/src/main/router/index.ts | Adds /topology redirect and new Topology route. |
| ui/src/main/App.vue | Route-scoped layout bounding/fill classes + related CSS for topology route. |
| ui/src/components/Topology/ViewManager.vue | New saved-views catalog dialog UI for open/rename/delete. |
| ui/src/components/Topology/TopologyPalette.vue | New draggable node palette with search/category filtering and virtual scrolling. |
| ui/src/components/Topology/TopologyLocationMap.vue | New Leaflet-based mini-map for node location in inspector. |
| ui/src/components/Topology/sources.ts | Builds topology source menu options from Graph API + curated overrides. |
| ui/src/components/Topology/severity.ts | Severity→color mapping + alarm aggregation utilities. |
| ui/src/components/Topology/search.ts | Node/category search utilities for topology. |
| ui/src/components/Topology/nodeIds.ts | Shared canvas ID conventions and parsing helpers. |
| ui/src/components/Topology/nodeActions.ts | Generates cross-links to node details/events/alarms/resource graphs. |
| ui/src/components/Topology/linkHints.ts | Pure logic for discovered adjacency “ghost link” hints and bindings. |
| ui/src/components/Topology/layout.ts | Auto-layout (d3-force / ForceAtlas2 / hierarchy) and edge curvature computation. |
| ui/src/components/Topology/interfaceState.ts | IF-MIB state naming + provenance/age description helpers. |
| ui/src/components/Topology/focus.ts | Focus-subgraph computation and default focus selection logic. |
| ui/src/components/Topology/dragTypes.ts | Defines palette→canvas drag MIME type and payload shape. |
| ui/src/components/Topology/deviceIcons.ts | Client-side legacy-faithful icon resolution + glyph/badge composition. |
| ui/pnpm-lock.yaml | Locks new dependencies (sigma/graphology/dompurify/etc.). |
| ui/packages/onms-ui/src/types.ts | Adds OnmsTableRowClickEvent type. |
| ui/packages/onms-ui/src/index.ts | Exports new seam components/types. |
| ui/packages/onms-ui/src/components/OnmsVirtualScroller.vue | New wrapper around PrimeVue VirtualScroller with narrowed slot payload. |
| ui/packages/onms-ui/src/components/OnmsTieredMenu.vue | New wrapper around PrimeVue TieredMenu with popup defaults + exposed API. |
| ui/packages/onms-ui/src/components/OnmsTable.vue | Adds selectionMode prop pass-through and row-click forwarding. |
| ui/packages/onms-ui/src/components/OnmsSlider.vue | New wrapper around PrimeVue Slider (single-value) + ariaLabel handling. |
| ui/packages/onms-ui/src/components/OnmsSelectButton.vue | New wrapper around PrimeVue SelectButton (allowEmpty default false, change emits value). |
| ui/packages/onms-ui/src/components/OnmsSearchInput.vue | Adds fluid prop mapping to ensure input fills container when desired. |
| ui/packages/onms-ui/src/components/OnmsMultiSelect.vue | Adds showToggleAll/maxSelectedLabels prop mapping. |
| ui/packages/onms-ui/src/components/OnmsInputNumber.vue | Adds showButtons/buttonLayout prop mapping. |
| ui/packages/onms-ui/src/components/OnmsContextMenu.vue | New wrapper around PrimeVue ContextMenu (pointer-positioned). |
| ui/packages/onms-ui/src/components/OnmsColorPicker.vue | New composite color picker (swatch grid + spectrum behind toggle) with hex normalization. |
| ui/packages/onms-ui/src/components/OnmsButton.vue | Adds size prop mapping. |
| ui/packages/onms-ui/src/components/OnmsAutoComplete.vue | Adds completeOnFocus prop mapping. |
| ui/packages/onms-ui/README.md | Documents tranche-4 components and seam behavior changes. |
| ui/package.json | Adds new runtime deps (sigma, graphology, dompurify, sigma plugins). |
| ui/eslint.config.js | Updates restricted PrimeVue imports to enforce new seam wrappers. |
| ui/CLAUDE.md | Updates seam coverage documentation and TieredMenu exception note. |
| opennms-webapp-rest/src/main/webapp/WEB-INF/menu/menu-template.json | Adds “Topology (Preview)” menu entry pointing at the new UI route. |
Files not reviewed (1)
- ui/pnpm-lock.yaml: Generated file
Suppressed comments (1)
ui/tests/containers/AppLayoutClasses.test.ts:89
- This test mounts
Appbut never unmounts the wrapper, which can leave DOM/router state behind for later tests. Unmounting at the end keeps the suite isolated.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
dino2gnt
left a comment
There was a problem hiding this comment.
I don't care if it eats babies and kills your pets, let's get it out there for feedback and fixes until it's good enough
A Vue replacement for the Vaadin topology map, under Topology (Preview). Hand-composed views: drag nodes from a palette of real OpenNMS nodes onto a sigma.js canvas, move and multi-select them, draw links, add free-standing labels and annotation shapes, undo and redo, then save the result as a named view. Views persist through /api/v2/topology/views, which stores the canvas as an opaque JSON document, so the service maps between that and the flat front-end model. A view is bookmarkable, and the catalog is shared rather than per-user. Discovered views: enlinkd's graphs served through /api/v2/graphs, grouped into Layer 2 and Layer 3 with a variant picker rather than the eleven flat namespaces the API reports, plus Path Outage as a derived source with a tiered hierarchy layout, being a rooted tree rather than a mesh. Focus and Semantic Zoom Level reduce a large graph to a node's neighborhood and both live in the URL. The source menu is built from the API rather than hardcoded, so an operator's GraphML topology or a bridged legacy provider appears without a UI change. Node color follows alarm severity, polled in View mode and frozen in Edit mode so the canvas does not repaint while arranging. Device icons derive from sysObjectId the way the legacy map did, and a view can override them per node. Selection drives an Inspector carrying the legacy map's Node and Technical Details plus the operator's own etc/infopanel templates; a vertex that is not an OnmsNode, such as an application or a service, is legible rather than showing a bare id. An Explore panel below lists the view's alarms, nodes, and -- on the application graph -- applications and perspective outages, with counts that follow the selection. Assisted composition: ghost links show discovered adjacencies between placed nodes that have no link yet, and a neighbor tray offers a selected node's discovered neighbors for placement. Every PrimeVue call site goes through @opennms/onms-ui, which this extends with five new wrappers and widens five existing ones, so the page holds no direct primevue/* import. Dark mode applies the theme class to <html> so PrimeVue's own component variables resolve, and the canvas-drawn colors CSS cannot reach are derived from the active theme.
Several faults compounded on a topology big enough to matter, and each one hid the next. Every id-filtered query built one FIQL clause per node, which on a 3454-vertex view exceeded Jetty's requestHeaderSize (4000 bytes by default, covering the whole request line and headers). The server answered 414 and each caller's catch turned that into "no data": an uncolored canvas, no device glyphs, empty Explore tabs. Queries are chunked by encoded length now, not by clause count -- counting cannot work, since node.id== is five bytes longer per clause than id== and axios percent-encodes = as %3D. Measured live, 150 node.id== clauses encode to 3011 bytes and pass while 200 encode to 4011 and answer 414. getNodeIconIds was never chunked at all. The large-graph gate tested whether a focus existed rather than how much would be drawn, so stepping the zoom twice from its own suggested anchor rendered ~1700 and then all 3414 vertices -- seconds of blocking layout, which is what the gate is for. It measures the rendered subgraph now, and offers the most-connected vertex as a starting point rather than leaving the user to guess a node name. Degree is structural on purpose: the API does carry each graph's defaultFocus, but the enlinkd value behind it is a global highest-summed-ifSpeed node and setDefaultVertex silently substitutes an arbitrary vertex when that node is absent, with nothing in the response to tell the two apart. Status and icons followed the whole placed-node set rather than what is on screen, so a focused view kept the previous subgraph's severities -- default blue in View mode, and indefinitely in Edit mode where the poll is stopped. The page also sized itself with calc(100vh - 104px), a constant measured once against the app shell that has to match the footer's rendered height. It was quietly the only thing bounding the page, since .app-layout is min-height:100vh, so a tall inspector grew the layout and pushed the footer off screen. The route bounds the layout and takes its height from the grid row instead, with no arithmetic. Discovered structure is refetched on demand from the toolbar, rather than polled: every enlinkd rescan interval defaults to 86400000 ms, and the button only ever called refreshStatus, which recolors what is already drawn. Tests assert encoded bytes rather than clause counts, which is the assertion that was missing when a first attempt at the chunking shipped still broken.
Search matched a node's label or its id, so an operator who knows a device by address could not find it, and the box only existed on discovered views. Every vertex already arrives with its provider properties, so one matcher covers what the old map needed a search provider apiece for: IpLike via the ipAddress enlinkd sets, plus an application's or a GraphML author's own attributes. Suggestions show the matched value, since a hit on an address is otherwise indistinguishable from a hit on a name. The box renders on custom views too, where picking a result selects the node and pans to it rather than focusing, there being no subgraph to reduce. Category was the old map's common way in, and the one search provider whose data the graph payload does not carry. A category hit resolves to the canvas ids of its members that are vertices of the current graph and selects them, which the canvas already highlights and the Explore panel already filters by. Categories come off the node payload because v2 /nodes cannot filter on them: its categories alias join is commented out in NodeRestService, so any category-prefixed filter answers 500 from Hibernate, and /api/v2/categories is a 404. The fetch is keyed on the node id set, dedupes concurrent callers, caches only success, and a generation counter stops a slow query landing on top of a newer one. Three providers were describing their own topology and the UI discarded most of it. VMware rendered every vertex as an identical circle, because our icon map is keyed on sysObjectId and its vmware.* keys matched nothing; the Vaadin map drew five distinct icons here. Two plumbing faults sat behind that: the discovered addNode never passed the vertex's icon, and icon resolution sat inside the resolvedNodeId branch, so a datacenter, network or datastore could never get one. GraphML's preferred-layout was ignored, so a layer declaring Hierarchy Layout drew force-directed, and its edge labels were dropped wholesale -- restoring them showed why, since enlinkd and VMware label every edge with its own namespace-qualified id. Selecting a link showed only its endpoints. The old map colored the line red when an interface was down and left the operator to work out which end; naming them is the more useful half and needs no alarm configuration. enlinkd reports the ifIndex inside the port's display string and its interface URL rather than as a field, which is why an earlier reading concluded no API exposed it per edge. Extracting it found two live bugs: parseNeighborNodeId took the first URL in a record, but enlinkd puts the local port's first, so every link resolved back to its own node and was dropped as a self-link; and the seen set was shared across protocols, so whichever parsed first discarded the rest, including the ports and ifIndex this exists to show.
…found
Restores the small map the Vaadin info panel put beside a selected node, when an
operator has filled in its asset coordinates. Nearly free once the data path was
checked: the UI already ships leaflet, and /api/v2/nodes/{id} carries
assetRecord.latitude/longitude, which the inspector already fetches. Titled
"Geographic Location" because Node Details already has a Location row meaning the
monitoring location. Absent coordinates, half a pair, and 0,0 all render nothing
-- Number(null) is 0, not NaN, and the API sends unset asset fields as null, so a
node with only a longitude was being plotted on the equator. The marker passes an
explicit icon, since Vite inlines marker-icon.png as a data URI and Leaflet's
default icon locates its images by parsing a CSS url.
Interface state on a link, the half of edge status that was designed and not
built: the raw IF-MIB ifOperStatus named rather than reduced to up/down, because
lowerLayerDown and down mean different things and up-values/down-values are
per-package config the API does not expose. Shown with its provenance, since
provisiond's node scan writes these columns daily and the SNMP Interface Poller
-- off by default, and needing ENABLE_POLLING per interface -- is the only thing
that makes them fresher.
Theming. The topology accent was a hardcoded literal in 26 places that never
followed the theme, measuring 3.07:1 against the dark canvas and 2.76:1 on a dark
panel, below the 3.0:1 floor WCAG 1.4.11 sets for non-text UI components. It is a
token pair now, and the nine persisted literals collapsed to DEFAULT_NODE_COLOR,
which stays theme-independent on purpose: a view composed in dark mode must not
save different colors from the same view in light mode. That separation is what
let link emphasis be fixed -- because the base color cannot follow the theme, the
light theme's highlight was the *same color* as the base and emphasis was carried
by fading everything else, which erased the other links at 1.20:1. Links are
drawn neutral now and the accent means "this one". Export follows the canvas
rather than a hardcoded white, which flattened a dark view while its labels kept
their dark-theme color at 1.29:1.
Defects an adversarial review found, all confirmed before fixing: a 500px
min-height on the canvas and its wrapper gave the page an unshrinkable floor, so
a viewport under ~1190px overflowed; the Explore panel refetched with no in-flight
guard, so a zoom step overlapped the status poll and the later response won; the
inspector assigned node detail after its await with no staleness check, so
clicking one node then another could leave the first's detail under the second;
IS-IS reports its index as a number with no port string and bridge keeps its far
ends in a nested array, so neither resolved an ifIndex; the location map never
called invalidateSize on resize; resolveDeviceIcon ran per node per frame,
linear-scanning ~90 keys on a miss that 3414 of 3454 vertices take; a cleared
alarm stuck and painted a node grey; and getNeighborsFor cached an empty result,
so one transient error hid a node's links for the session.
The search box carries the focused node, so the hop stepper reads as "2 hops from
this node". The body margin reset is scoped to this route rather than applied to
<body>, which had also stripped every other page's gutters and restyled every
teleported PrimeVue overlay.
Adds the tests review found missing: gate re-arming per load, the "Render all"
click path, App.vue's route-scoped classes, and getNodeCategories, which was
mocked everywhere and never exercised.
All three were real, though one was smaller than reported. `buildSources` keyed its claimed-namespace set with a literal NUL byte. NUL is the right separator semantically, since it cannot occur in a container id or a namespace, but as a raw byte in source it is invisible and easy for tooling to strip -- it rendered as a space in every reading of that file, including mine. It is an explicit escape now, and a test constructs the collision the separator prevents: a container id ending where a curated namespace begins keyed to the same string and had its graphs silently dropped from the menu. `computeEdgeCurvatures` tests every link against every node. Measured at the real Layer 2 graph, 3454 nodes and 3501 links, that is 325ms -- noticeable but an order of magnitude below the ForceAtlas2 layout it accompanies, and reached only once the user has accepted the large-graph gate, so "freezing the page" overstates it. Skipped past a ~2M node-link budget regardless: at that size links are hairline and curving them buys no clarity. The App layout test mounted without tearing down. Same leak that was fixed in the container tests earlier in this branch, reintroduced in a new file: a live App keeps its router watchers, so a later test's navigation drives a stale instance.
23fdbc9 to
ad64c64
Compare
|
ran a quick test and found a couple of minor issues,
|
Thank you! What browser? |
Edge |
synqotik
left a comment
There was a problem hiding this comment.
Seems reasonable to me. Since it's a preview we can always fix/iterate as needed.
…n canvas Double-clicking empty canvas creates a label and enters edit mode immediately. The edit input was a fixed 12ch wide, and the label box is centered on the click point, so an empty new label put its caret about 6ch left of the cursor. An existing label hugs its text and looked right, which is why this showed only for the case in the report. The input starts at 4ch and grows with the text. Shape drawing listens for mousemove on `window` so the release still arrives when the pointer leaves the canvas, but it used those coordinates raw: dragging past an edge converted to graph points outside the visible area. Start and move are clamped to the canvas rect now. The preview also measured against the draw overlay's rect while the commit measured against the canvas's -- they coincide today, both layers being inset:0 on the same parent, so this was latent rather than the reported symptom, but the comments already flagged the mismatch. Both use the same rect, so the preview cannot drift from the shape it creates. The clamp lives in layout.ts as a pure helper rather than inside the canvas component, which has no tests of its own because sigma touches WebGL on import.
New, Save As and Rename each asked for a name with window.prompt. Beyond looking nothing like the rest of the page, that carried a real fault: once a user ticks "prevent this page from creating additional dialogs" -- offered by Chromium after a couple of prompts, and easy to hit while creating several views -- every later prompt is suppressed and returns null, which all three read as "cancelled". The buttons then did nothing at all, with no error, until the page was reloaded. One ViewNameDialog serves all three. The mode decides the wording, what the field is seeded with, and what the answer does, so the collision rule stays where the difference actually lives: Save As has to create a new entry, so the open view's own name is a conflict, while Rename may keep it and treat it as a no-op. The collision check moves ahead of the request rather than following it. prompt could only close and then toast; the dialog disables the action, names the view already using the name, and lets the user edit without retyping the rest. The toast is still there for the case the check cannot cover, a catalog gone stale against another session. OnmsDialog gains a `show` re-emit. Focus has to move into the field once it is in the DOM, and the dialog appends to body behind a transition, so mount and nextTick are both too early.
A standalone catalog browser with list, rename and delete. Nothing imported it: the only reference outside its own test was two comments naming it as a caller. Topology.vue grew its own rename and delete against the same store actions instead, so the page it was meant to be never got built. Left alone it would have been the last window.prompt and window.confirm in the tree, and worth a dialog nobody could open. The store actions and the catalog type stay: the view chooser and the collision check use both.
|
Updates in for you to review @mershad-manesh |
A box drawn on a view with no nodes was stored at a position and size the renderer then drew somewhere else, so it jumped and resized the moment the drag ended. Sigma's viewportToGraph and graphToViewport are not inverses of each other here: with a custom bounding box and nothing to normalise from, one scales both axes by the box-to-viewport ratio while the other stretches each axis independently. Measured on a blank view, a 547x340 drag at 395,101 came back as 422x170 at 859,50. The conversion now measures where three graph points land in the viewport and inverts that, so it is the exact inverse of the projection the renderer uses whatever sigma does internally. A view with nodes was unaffected and stays so, since the calibration reduces to the same mapping. Found by instrumenting the round trip on screen. Two attempts before that reasoned about reflow from the toolbar's button label and the inspector, and were wrong: the captured numbers showed the projection never changes after the drag, only that it disagrees with the one used to store the shape.
Both topology entries declared network/Instances, so the two were indistinguishable in the sidebar. The preview takes network/Nodes, which reads as points converging on a centre rather than borrowing the chassis metaphor the other network icons use. Only menu-template.json carries this entry, so the other menu templates need nothing. Icons resolve through an explicit import and switch in useMenuIcons, not a glob, and an unregistered id falls back silently to the home icon; network/Nodes is already mapped there.
This is the much worked on Topology Preview UI for H36/M2026. Almost all of this is UI work. I think this likely still needs docs but those can wait until we can get screenshots. There is a high chance that the footer is going to cause issues but I've validated it as much as I can in different browsers and resolutions.
I've made some choices for rendering around number of nodes in the view that I think make sense so this is scalable in the long term. If you test with >3000 connected nodes then you'll see them.... only time will tell if those are good choices.
I would love feedback. If you need test data then let me know.
Assisted by Anthropic Claude Opus 4.8, 5, and Fable 5.
External References