fix(Canvas): Fix CustomNode rerendering without controller

packages/ui-tests/cypress/e2e/designer/specialStepConfiguration/expressionStepConfig.cy.ts

@@ -25,34 +25,38 @@ describe('Tests for sidebar expression configuration', () => {
     cy.uploadFixture('flows/camelRoute/basic.yaml');
     cy.openDesignPage();
 
-    cy.openStepConfigurationTab('setHeader');
+    cy.openStepConfigurationTabByPath('custom-node__route.from.steps.0.setHeader');
     cy.selectFormTab('All');
     cy.selectExpression('JQ');
     cy.interactWithConfigInputObject('expression', '.id');
     cy.addExpressionResultType('java.lang.String');
     cy.interactWithConfigInputObject('trim');
 
+    // TODO: Closing the configuration panel because adding a new step keep the selection status,
+    // but closes the panel. This will be fixed in https://github.com/KaotoIO/kaoto/issues/1923
+    cy.closeStepConfigurationTab();
+
     cy.selectAppendNode('setHeader');
     cy.chooseFromCatalog('processor', 'setHeader');
 
     cy.checkNodeExist('setHeader', 2);
 
-    cy.openStepConfigurationTab('setHeader', 1);
+    cy.openStepConfigurationTabByPath('custom-node__route.from.steps.1.setHeader');
     cy.selectFormTab('All');
     cy.selectExpression('JQ');
     cy.interactWithConfigInputObject('expression', '.name');
     cy.addExpressionResultType('java.lang.String');
     cy.interactWithConfigInputObject('trim');
 
-    cy.openStepConfigurationTab('setHeader', 0);
+    cy.openStepConfigurationTabByPath('custom-node__route.from.steps.0.setHeader');
 
     // Check the configured fields didn't disappear from the first node
     cy.checkConfigCheckboxObject('trim', true);
     cy.checkExpressionResultType('java.lang.String');
     cy.checkConfigInputObject('expression', '.id');
 
     // Check the configured fields didn't disappear from the second node
-    cy.openStepConfigurationTab('setHeader', 0);
+    cy.openStepConfigurationTabByPath('custom-node__route.from.steps.1.setHeader');
     cy.checkConfigCheckboxObject('trim', true);
     cy.addExpressionResultType('java.lang.String');
     cy.checkConfigInputObject('expression', '.name');

packages/ui-tests/cypress/support/cypress.d.ts

@@ -46,6 +46,7 @@ declare global {
       // design
       openGroupConfigurationTab(step: string, stepIndex?: number): Chainable<JQuery<Element>>;
       openStepConfigurationTab(step: string, stepIndex?: number): Chainable<JQuery<Element>>;
+      openStepConfigurationTabByPath(path: string): Chainable<JQuery<Element>>;
       toggleExpandGroup(groupName: string): Chainable<JQuery<Element>>;
       fitToScreen(): Chainable<JQuery<Element>>;
       closeStepConfigurationTab(): Chainable<JQuery<Element>>;

packages/ui-tests/cypress/support/next-commands/design.ts

@@ -7,6 +7,10 @@ Cypress.Commands.add('openStepConfigurationTab', (step: string, stepIndex?: numb
   cy.get(`g[data-nodelabel^="${step}"]`).eq(stepIndex).click({ force: true });
 });
 
+Cypress.Commands.add('openStepConfigurationTabByPath', (path: string) => {
+  cy.get(`g[data-testid="${path}"]`).click({ force: true });
+});
+
 Cypress.Commands.add('openGroupConfigurationTab', (group: string, groupIndex?: number) => {
   groupIndex = groupIndex ?? 0;
   cy.get(`g[data-grouplabel^="${group}"]`).eq(groupIndex).click({ force: true });

packages/ui/src/components/Visualization/Canvas/Canvas.tsx

@@ -90,8 +90,16 @@
       },
     };
 
-    controller.fromModel(model, false);
-    setInitialized(true);
+    if (!initialized) {
+      controller.fromModel(model, false);
+      setInitialized(true);
+      return;
+    }
+
+    requestAnimationFrame(() => {
+      controller.fromModel(model, true);
+      controller.getGraph().layout();
+    });
   }, [controller, entities, visibleFlows]);
 
   const handleSelection = useCallback((selectedIds: string[]) => {

packages/ui/src/components/Visualization/Canvas/controller.service.ts

@@ -12,7 +12,7 @@
   withPanZoom,
 } from '@patternfly/react-topology';
 import { CustomGroupWithSelection, CustomNodeWithSelection, NoBendpointsEdge } from '../Custom';
-import { PlaceholderNode } from '../Custom/Node/PlaceholderNode';
+import { PlaceholderNodeWithDnD } from '../Custom/Node/PlaceholderNode';
 import { LayoutType } from './canvas.models';
 import { CustomEdge } from '../Custom/Edge/CustomEdge';
 
@@ -52,7 +52,7 @@
       case 'group':
         return CustomGroupWithSelection;
       case 'node-placeholder':
-        return PlaceholderNode;
+        return PlaceholderNodeWithDnD;
       default:
         switch (kind) {
           case ModelKind.graph:

packages/ui/src/components/Visualization/Custom/Group/CustomGroup.tsx

@@ -7,35 +7,55 @@
   withSelection,
 } from '@patternfly/react-topology';
 import { FunctionComponent } from 'react';
-import { CanvasDefaults } from '../../Canvas/canvas.defaults';
 import { CanvasNode } from '../../Canvas/canvas.models';
 import { NodeContextMenuFn } from '../ContextMenu/NodeContextMenu';
-import { CustomGroupCollapsible } from './CustomGroupCollapsible';
+import { CustomNodeObserver } from '../Node/CustomNode';
+import { useCollapseStep } from '../hooks/collapse-step.hook';
+import { CustomGroupExpanded } from './CustomGroupExpanded';
 
 type IDefaultGroup = Parameters<typeof DefaultGroup>[0];
 interface ICustomGroup extends IDefaultGroup {
   element: GraphElement<CanvasNode, CanvasNode['data']>;
 }
 
-const CustomGroup: FunctionComponent<ICustomGroup> = observer(({ element, ...rest }) => {
-  const vizNode = element.getData()?.vizNode;
-  const label = vizNode?.getNodeLabel();
-
+const CustomGroupInner: FunctionComponent<ICustomGroup> = observer(({ element, onCollapseChange, ...rest }) => {
   if (!isNode(element)) {
-    throw new Error('CustomGroup must be used only on Node elements');
+    throw new Error('CustomGroupInner must be used only on Node elements');
+  }
+
+  const { onCollapseNode, onExpandNode } = useCollapseStep(element);
+
+  if (element.isCollapsed()) {
+    return (
+      <CustomNodeObserver
+        {...rest}
+        element={element}
+        onCollapseToggle={() => {
+          onExpandNode();
+          onCollapseChange?.(element, true);
+        }}
+      />
+    );
   }
 
   return (
-    <CustomGroupCollapsible
+    <CustomGroupExpanded
       {...rest}
       element={element}
-      label={label}
-      collapsible
-      collapsedWidth={CanvasDefaults.DEFAULT_NODE_WIDTH}
-      collapsedHeight={CanvasDefaults.DEFAULT_NODE_HEIGHT}
-      hulledOutline={false}
+      onCollapseToggle={() => {
+        onCollapseNode();
+        onCollapseChange?.(element, false);
+      }}
     />
   );
 });
 
+const CustomGroup: FunctionComponent<ICustomGroup> = ({ element, ...rest }: ICustomGroup) => {
+  if (!isNode(element)) {
+    throw new Error('CustomGroup must be used only on Node elements');
+  }
+
+  return <CustomGroupInner element={element} {...rest} />;
+};
+
 export const CustomGroupWithSelection = withSelection()(withContextMenu(NodeContextMenuFn)(CustomGroup));

packages/ui/src/components/Visualization/Custom/Group/CustomGroupExpanded.tsx

@@ -11,7 +11,6 @@
   observer,
   useAnchor,
   useHover,
-  useSelection,
   withDndDrop,
 } from '@patternfly/react-topology';
 import { FunctionComponent, useContext, useRef } from 'react';
@@ -21,13 +20,13 @@
 import { StepToolbar } from '../../Canvas/StepToolbar/StepToolbar';
 import { CanvasDefaults } from '../../Canvas/canvas.defaults';
 import { AddStepIcon } from '../Edge/AddStepIcon';
+import { customGroupExpandedDropTargetSpec } from '../customComponentUtils';
 import { TargetAnchor } from '../target-anchor';
 import './CustomGroupExpanded.scss';
 import { CustomGroupProps } from './Group.models';
-import { customGroupExpandedDropTargetSpec } from '../customComponentUtils';
 
 export const CustomGroupExpandedInner: FunctionComponent<CustomGroupProps> = observer(
-  ({ element, onContextMenu, onCollapseToggle, dndDropRef, droppable }) => {
+  ({ element, onContextMenu, onCollapseToggle, dndDropRef, droppable, selected, onSelect }) => {
     if (!isNode(element)) {
       throw new Error('CustomGroupExpanded must be used only on Node elements');
     }
@@ -37,7 +36,6 @@
     const label = vizNode?.getNodeLabel(settingsAdapter.getSettings().nodeLabel);
     const isDisabled = !!vizNode?.getComponentSchema()?.definition?.disabled;
     const tooltipContent = vizNode?.getTooltipContent();
-    const [isSelected, onSelect] = useSelection();
     const [isGHover, gHoverRef] = useHover<SVGGElement>(CanvasDefaults.HOVER_DELAY_IN, CanvasDefaults.HOVER_DELAY_OUT);
     const [isToolbarHover, toolbarHoverRef] = useHover<SVGForeignObjectElement>(
       CanvasDefaults.HOVER_DELAY_IN,
@@ -46,8 +44,8 @@
     const boxRef = useRef<Rect | null>(null);
     const shouldShowToolbar =
       settingsAdapter.getSettings().nodeToolbarTrigger === NodeToolbarTrigger.onHover
-        ? isGHover || isToolbarHover || isSelected
-        : isSelected;
+        ? isGHover || isToolbarHover || selected
+        : selected;
     const shouldShowAddStep =
       shouldShowToolbar && vizNode?.getNodeInteraction().canHaveNextStep && vizNode.getNextNode() === undefined;
     const isHorizontal = element.getGraph().getLayout() === LayoutType.DagreHorizontal;
@@ -80,7 +78,7 @@
           className="custom-group"
           data-testid={`custom-group__${vizNode.id}`}
           data-grouplabel={label}
-          data-selected={isSelected}
+          data-selected={selected}
           data-disabled={isDisabled}
           data-toolbar-open={shouldShowToolbar}
           onClick={onSelect}

packages/ui/src/components/Visualization/Custom/Node/CustomNode.test.tsx

@@ -0,0 +1,45 @@
+import { BaseEdge, BaseGraph, BaseNode, ElementContext, VisualizationProvider } from '@patternfly/react-topology';
+import { act, render } from '@testing-library/react';
+import { TestProvidersWrapper } from '../../../../stubs';
+import { ControllerService } from '../../Canvas/controller.service';
+import { CustomNodeObserver } from './CustomNode';
+
+describe('CustomNode', () => {
+  afterEach(() => {
+    jest.restoreAllMocks();
+  });
+
+  it('should throw an error if not used on Node elements', () => {
+    jest.spyOn(console, 'error').mockImplementation(() => {});
+    const edgeElement = new BaseEdge();
+
+    expect(() => {
+      act(() => {
+        render(<CustomNodeObserver element={edgeElement} />);
+      });
+    }).toThrow('CustomNode must be used only on Node elements');
+  });
+
+  it('should render without error', () => {
+    const parentElement = new BaseGraph();
+    const element = new BaseNode();
+    const controller = ControllerService.createController();
+    parentElement.setController(controller);
+    element.setController(controller);
+    element.setParent(parentElement);
+
+    const { Provider } = TestProvidersWrapper();
+
+    const wrapper = render(
+      <Provider>
+        <VisualizationProvider controller={controller}>
+          <ElementContext.Provider value={element}>
+            <CustomNodeObserver element={element} />
+          </ElementContext.Provider>
+        </VisualizationProvider>
+      </Provider>,
+    );
+
+    expect(wrapper.asFragment()).toMatchSnapshot();
+  });
+});