Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Canvas): Add dynamic labels to Canvas' nodes #556

Merged
merged 1 commit into from
Dec 15, 2023

Conversation

lordrip
Copy link
Member

@lordrip lordrip commented Dec 14, 2023

Context

Currently, the node's label is set once while creating the graph.

Changes

This pull request changes this behavior by reading the label on each node, by leveraging the new getLabel method in IVisualizationNode, CamelRouteVisualEntity and PipeVisualEntity classes.

By removing the label property from the data object from VisualizationNode, the node id become more relevant.

Screenshots

Node with a short description

Screenshot from 2023-12-14 15-47-25

Node with a long description

Screenshot from 2023-12-14 15-48-18

A longer description on hover

Screenshot from 2023-12-14 15-48-30

fix: #483

@lordrip
Copy link
Member Author

lordrip commented Dec 14, 2023

@ all sorry for the noisy diff, but since we're removing the label property from the CanvasNode interface, it means that we need to reshape all places where the nodes are created 🥲

@lordrip lordrip force-pushed the feat/add-meaningful-labels branch 2 times, most recently from 286af88 to 45630b1 Compare December 14, 2023 15:00
@@ -208,14 +207,10 @@ export class CanvasService {
return edges;
}

private static getNode(
id: string,
options: { label?: string; parentNode?: string; data?: CanvasNode['data'] } = {},
Copy link
Member Author

Choose a reason for hiding this comment

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

Removing label from the getNode method

<DefaultNode element={element} showStatusDecorator {...rest}>
<g data-testid={`custom-node__${vizNode?.id}`}>
<DefaultNode element={element} label={label} truncateLength={15} {...rest} showStatusDecorator>
<g data-testid={`custom-node__${vizNode?.id}`} data-nodelabel={label}>
Copy link
Member Author

Choose a reason for hiding this comment

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

@tplevko Since long labels might be truncated, the entire label will be held in the data-nodelabel attribute, I updated the e2e commands to reflect this.

if (camelElementLookup.componentName !== undefined) {
return camelElementLookup.componentName;
}

const uriString = getUriString(definition);
Copy link
Member Author

Choose a reason for hiding this comment

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

Streamlined how to extract the uri string, considering whether the definition is a string or if it's an object with a uri property in it. It also handles empty string cases.

@@ -141,11 +146,15 @@ export class CamelComponentSchemaService {
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
private static getCamelElement(processorName: keyof ProcessorDefinition, definition: any): ICamelElementLookupResult {
if (!isDefined(definition)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

If there's no node definition, it's probably that the node no longer exists, so better to fallback to the processorName

@lordrip lordrip force-pushed the feat/add-meaningful-labels branch from 45630b1 to 804f4c7 Compare December 14, 2023 15:23
@lordrip lordrip requested a review from igarashitm December 14, 2023 15:35
@igarashitm
Copy link
Contributor

That inline? hover looks nice 👍

Currently, the node's label is set once while creating the graph.

This commit changes this behavior by reading the label on each node, by
leveraging the new `getLabel` method in `IVisualizationNode`, `CamelRouteVisualEntity`
and `PipeVisualEntity` classes.

By removing the `label` property from the `data` object from
`VisualizationNode`, the node id become more relevant.

fix: KaotoIO#483
@lordrip lordrip force-pushed the feat/add-meaningful-labels branch from 804f4c7 to 92bbd87 Compare December 15, 2023 13:38
@lordrip lordrip merged commit 6ab3a82 into KaotoIO:main Dec 15, 2023
6 checks passed
@lordrip lordrip deleted the feat/add-meaningful-labels branch December 15, 2023 14:18
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.

Show meaningful node labels on the canvas
2 participants