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): Improve add steps into canvas #1673

Merged
merged 3 commits into from
Dec 3, 2024

Conversation

lordrip
Copy link
Member

@lordrip lordrip commented Nov 29, 2024

Context

Currently, we have 3 alternative mechanisms for adding new steps in the canvas:

  1. Right-click on a node and select between Prepend, Append, and
    Insert.
  2. Use the Step Toolbar to Insert
  3. Use the quick add edge.

Changes

This commit replaces the quick add edge with a floating button over the last element of the route. In addition to that, it generates a placeholder when the steps property is empty, so the user can quickly add a new step to initialize the branch.

Notes

  1. This PR requires chore(Canvas): Extract onAddStep hook #1621 to be merged first
  2. The right-click node interaction is still preserved

fix: #1648
fix: #1647
fix: #1072

@lordrip lordrip force-pushed the feat/add-step-placeholder branch 4 times, most recently from f90dd63 to d376281 Compare December 2, 2024 11:45
@lordrip lordrip changed the title Feat/add step placeholder feat(Canvas): Improve add steps into canvas Dec 2, 2024
@lordrip lordrip force-pushed the feat/add-step-placeholder branch 2 times, most recently from b4e1fd0 to 8cf7b33 Compare December 2, 2024 12:43
@KaotoIO KaotoIO deleted a comment from codecov bot Dec 2, 2024
@lordrip lordrip force-pushed the feat/add-step-placeholder branch from 8cf7b33 to e911ad6 Compare December 3, 2024 09:12
Copy link

codecov bot commented Dec 3, 2024

Codecov Report

Attention: Patch coverage is 77.61194% with 30 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@7e23d8f). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...ents/Visualization/Custom/Node/PlaceholderNode.tsx 41.66% 12 Missing and 2 partials ⚠️
...mponents/Visualization/Custom/Edge/AddStepIcon.tsx 69.23% 4 Missing ⚠️
...omponents/Visualization/Custom/Edge/CustomEdge.tsx 90.62% 3 Missing ⚠️
...Visualization/Custom/Group/CustomGroupExpanded.tsx 78.57% 3 Missing ⚠️
...ponents/Visualization/Canvas/controller.service.ts 80.00% 2 Missing ⚠️
...ualization/flows/nodes/mappers/base-node-mapper.ts 77.77% 2 Missing ⚠️
...omponents/Visualization/Custom/Node/CustomNode.tsx 88.88% 1 Missing ⚠️
...c/models/visualization/flows/pipe-visual-entity.ts 75.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1673   +/-   ##
=======================================
  Coverage        ?   84.32%           
  Complexity      ?      365           
=======================================
  Files           ?      287           
  Lines           ?     8208           
  Branches        ?     1616           
=======================================
  Hits            ?     6921           
  Misses          ?     1148           
  Partials        ?      139           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@lhein lhein left a comment

Choose a reason for hiding this comment

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

tested only functionality on web browser and it works as expected

Currently, we have 3 alternative mechanisms for adding new steps in the
canvas:

1. Right click on a node and select between `Prepend`, `Append`, and
   `Insert`.
2. Use the Step Toolbar to `Insert`
3. Use the quick add edge.

This commit replaces the quick add edge with a floating button over the
last element of the route. In addition to that, it generates a
placeholder when the steps property is empty, so the user can quickly
add a new step to initialize the branch.
@lordrip lordrip force-pushed the feat/add-step-placeholder branch from e911ad6 to 1735e95 Compare December 3, 2024 12:40
@@ -141,7 +141,7 @@ export abstract class AbstractCamelVisualEntity<T extends object> implements Bas
/** If we're in Replace mode, we need to delete the existing step */
const deleteCount = options.mode === AddStepMode.ReplaceStep ? 1 : 0;

const stepsArray: ProcessorDefinition[] = getValue(this.entityDef, pathArray.slice(0, -2), []);
Copy link
Member Author

Choose a reason for hiding this comment

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

This was the root cause of the bug preventing adding steps in EIPs with steps: undefined, like multicast or filter.

The problem was that for array properties (like steps) that don't exist by default, for instance, when adding a multicast, aggregate or filter, Kaoto doesn't offer a template with the steps property already created, the code creates a new array but it doesn't assign it to the object, therefore, the step gets assigned to a newly created array completely detached from the Camel Route structure.

The fix is to use the getArrayProperty which ensures the requested array, regardless of whether it exists or not, is properly attached to the structure.

@lordrip lordrip added this to the 2.3.0 milestone Dec 3, 2024
Since the StepToolbar `+` button was removed in favor of using a
placeholder node or the `arrow` icon, we no longer need to disable the
node interactions for parallel EIPs like `multicast` and loadBalance`
Copy link

sonarcloud bot commented Dec 3, 2024

@lordrip lordrip marked this pull request as ready for review December 3, 2024 21:19
@lordrip lordrip merged commit a0d982b into KaotoIO:main Dec 3, 2024
14 checks passed
@lordrip lordrip deleted the feat/add-step-placeholder branch December 3, 2024 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants