Skip to content

Enable domain visualization on graph explorer#2810

Open
peeterskris wants to merge 14 commits intoconveyordata:mainfrom
peeterskris:feature/enable-domain-visualization-explorer
Open

Enable domain visualization on graph explorer#2810
peeterskris wants to merge 14 commits intoconveyordata:mainfrom
peeterskris:feature/enable-domain-visualization-explorer

Conversation

@peeterskris
Copy link
Contributor

Summary

  • Activate the existing but disabled domain container node infrastructure in the Explorer view
  • Synthesize domain parent nodes from the domain_id and domain fields already returned by the API, assigning each domain a distinct transparent color from an 8-color palette
  • Pass domainsEnabled through to parseRegularNode so child nodes are nested inside their domain boxes via parentId
  • Keep domain container nodes visible during node selection highlighting when any child is connected

Test plan

  • Open http://localhost:8080/explorer and confirm colored domain boxes appear around grouped data products/datasets
  • Confirm the sidebar Domains filter toggle hides/shows domain boxes
  • Confirm node selection highlighting still works — domain boxes stay visible when a child node is selected
  • Confirm ELK layout handles sizing/positioning cleanly

🤖 Generated with Claude Code

Activate the existing but disabled domain container node infrastructure
in the Explorer view. Synthesizes domain parent nodes from the domain_id
and domain fields already returned by the API, assigns each domain a
distinct transparent color, and passes domainsEnabled through so child
nodes are nested inside their domain boxes via parentId. Also keeps
domain containers visible during node selection highlighting.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Set elk.hierarchyHandling to INCLUDE_CHILDREN on both inter- and intra-
domain layout options so cross-domain edges influence the global left-to-
right layering. Also align the inter-domain options with the same quality
settings (NETWORK_SIMPLEX, LAYER_SWEEP, FIXED_SIDE ports) used by the
basic layout.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@stijndehaes
Copy link
Contributor

stijndehaes commented Mar 2, 2026

@peeterskris thank you for the contribution, however you have missed a few things by overriding our PR template.

We have the following checks:

  • Update the release notes document if needed in the release notes
  • When updating the UI please provide screenshots or videos to the reviewers

Can you please ensure you do both of these :)

Also a heads up, we are planning to improve this explorer view at some point. We are looking into what people actually want to achieve with this page, and what the job to be done is. If you can share what you do with this page, kindly let us know. It would be a ton of help!

Another annoying issue with the change here is that we had a couple of problems with the domain nodes in the past.
The first one begin performance issues, these were mostly backend so I don't think this introduces them again. But it would help if you can check that.
Another problem we had is that the view with the domain nodes was really cluttered and made it very hard to see certain things. So it's best if we can have a screenshot with a lot of nodes. An alternative to domain nodes could for example be to add colors to the current nodes , and have a legend to explain which color is which domain.

TODOs:

  • Share screenshot, we can then see how cluttered everything looks and continue the discussion
  • Measure performance (just measuring time to the fully rendered explorer view is good, this can also be done by providing a video)

@peeterskris
Copy link
Contributor Author

I loaded a fairly complex graph and it looks like this:
image
It loads in about 1s.
image

I can test on bigger graphs. Where does it hit performance bottlenecks?

Kris Peeters and others added 3 commits March 2, 2026 21:47
The sidebar gets setNodes directly from useReactFlow(), so passing it
as a prop from the parent was redundant and caused a TypeScript shadow.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a Switch control that sets domainsEnabled in SidebarFilters,
allowing users to show/hide domain container boxes. The plumbing was
already in place; this adds the missing UI control.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@peeterskris
Copy link
Contributor Author

In my opinion it is less cluttered with the domains enabled. But there is a toggle box.
image
image

peeterskris and others added 8 commits March 2, 2026 22:00
Backend: extend graph router tests to verify domain_id and domain fields
are populated on data product and output port nodes, and that multiple
domains are correctly distinguished.

Frontend: add node-parser.helper tests covering the domainsEnabled flag
— parentId is set when enabled and domain_id is present, and absent
otherwise.

Also fix TypeScript error (setNodes still passed as prop after removal)
and add missing 'Show Domains' translation key.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- node-parser.helper.test.ts: replace 'as any' cast with proper Node type
  and NodeType enum from the generated graph API types
- node-editor.tsx: remove redundant fragment wrapping single ReactFlow child

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Importing NodeType enum from graphApi transitively loads the Redux store
which calls AppConfig.getApiBaseURL() and crashes in the Node test env.
Use a biome-ignore suppression on the as-any cast in the test factory
instead, which is the right trade-off for a pure test helper.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
'import type' is erased at compile time so it does not trigger the
Redux store chain that reads AppConfig.getApiBaseURL(). Use string
literal cast to satisfy the NodeType enum field without importing
the enum value.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-visualization-explorer

# Conflicts:
#	docs/docs/release-notes.md
@stijn-janssens
Copy link
Collaborator

Scherm­afbeelding 2026-03-04 om 10 52 56 I am not sure if the behavior I'm seeing is the same you saw, but for me it all gets layed out very vertically when domains are enabled. Also for smaller not super interconnected datasets (maybe they need to be connected to make it work well, could be)

Copy link
Collaborator

@stijn-janssens stijn-janssens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it! An idea: Can we show the domains as coloured borders around nodes when domains are disabled? To give a sense of grouping at least without showing the groups themselves? Or is that overkill and do we want disabled domains to actually mean "no domains visible at all"?


function parseFullNodes(nodes: GraphNode[], setNodeId: (id: string) => void): Node[] {
const DOMAIN_COLORS = [
{ bg: 'rgba(59, 130, 246, 0.08)', border: 'rgba(59, 130, 246, 0.3)' }, // blue
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine for now, but maybe this should be logged as tech debt as this both hardcodes the colors and will recycle them after 8 domains.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already use antv from ant design, I believe there might be a colorpallette there. I will look into it and respond here again.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests should be placed in the tests folder

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No in front end testing, it is preferred to place it next to the component

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:o that feels weird :D But okay! Learned something new

- Move node-parser.helper.test.ts to src/tests/ folder per project convention
- Add aspect ratio and component compaction to ELK inter-domain layout to
  reduce excessive vertical stacking when domains are enabled

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
assert edge["target"] == str(consumer.id), (
"Edge target should be the consumer (dataset reader)"
)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unsure what the tests add to be honest, the logic did not change? So unsure why they are required. Can you explain to me the reasoning for adding these?

The includes domain fields tests for example could just be the last 2 asserts that are added to test_get_graph_data.


function parseFullNodes(nodes: GraphNode[], setNodeId: (id: string) => void): Node[] {
const DOMAIN_COLORS = [
{ bg: 'rgba(59, 130, 246, 0.08)', border: 'rgba(59, 130, 246, 0.3)' }, // blue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already use antv from ant design, I believe there might be a colorpallette there. I will look into it and respond here again.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No in front end testing, it is preferred to place it next to the component

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants