fix(viz): Avoid displaying the MiniCatalog if there are no steps or b…

…ranches available At this moment, trying to append an additional step after a step that has a custom extension, causes to display the MiniCatalog with both tabs disabled and showing the Add branch button enabled. This is problematic because for those steps, a special configuration is required and its provided via its extension, hence disabling the branch tab from the MiniCatalog. This commit disables the plus icon to avoid displaying a non interactive popup to the user and providing a tooltip instead explaining what to do in order to configure the step. This commit also target small improvements in related tests, like changing the api signature of one of the services to only receive a step instead of a whole node. fixes #1473
kaoto-archive · Mar 17, 2023 · 0b91a5b · 0b91a5b
1 parent 86cffb7
commit 0b91a5b
Show file tree

Hide file tree

Showing 10 changed files with 105 additions and 66 deletions.
diff --git a/src/components/AppendStepButton.test.tsx b/src/components/AppendStepButton.test.tsx
@@ -23,7 +23,6 @@ describe('AppendStepButton.tsx', () => {
           handleAddBranch={noopFn}
           handleSelectStep={noopFn}
           layout={'LR'}
-          showBranchesTab={true}
           showStepsTab={true}
           supportsBranching={true}
           step={kameletSourceStepStub}
@@ -42,7 +41,6 @@ describe('AppendStepButton.tsx', () => {
           handleAddBranch={noopFn}
           handleSelectStep={noopFn}
           layout={'LR'}
-          showBranchesTab={true}
           showStepsTab={true}
           supportsBranching={true}
           step={kameletSourceStepStub}
@@ -66,7 +64,6 @@ describe('AppendStepButton.tsx', () => {
           handleAddBranch={noopFn}
           handleSelectStep={noopFn}
           layout={'LR'}
-          showBranchesTab={false}
           showStepsTab={true}
           supportsBranching={true}
           step={{
@@ -98,14 +95,13 @@ describe('AppendStepButton.tsx', () => {
     });
   });
 
-  test('should disable branches tab when showBranchesTab={false} and supportsBranching={false}', async () => {
+  test('should disable branches tab when supportsBranching={false}', async () => {
     render(
       <AlertProvider>
         <AppendStepButton
           handleAddBranch={noopFn}
           handleSelectStep={noopFn}
           layout={'LR'}
-          showBranchesTab={false}
           showStepsTab={true}
           supportsBranching={false}
           step={kameletSourceStepStub}
@@ -142,7 +138,6 @@ describe('AppendStepButton.tsx', () => {
           handleAddBranch={noopFn}
           handleSelectStep={noopFn}
           layout={'LR'}
-          showBranchesTab={true}
           showStepsTab={true}
           supportsBranching={true}
           step={kameletSourceStepStub}
@@ -171,4 +166,36 @@ describe('AppendStepButton.tsx', () => {
 
     spy.mockReset();
   });
+
+  test('should disable the plus button when showStepsTab={false} and supportsBranching={false}', async () => {
+    const spy = jest.spyOn(StepsService, 'hasCustomStepExtension').mockReturnValue(true);
+
+    render(
+      <AlertProvider>
+        <AppendStepButton
+          handleAddBranch={noopFn}
+          handleSelectStep={noopFn}
+          layout={'LR'}
+          showStepsTab={false}
+          supportsBranching={false}
+          step={kameletSourceStepStub}
+        />
+      </AlertProvider>
+    );
+
+    const plusIcon = screen.getByTestId('stepNode__appendStep-btn');
+    expect(plusIcon).toBeDisabled();
+
+    act(() => {
+      fireEvent.mouseEnter(plusIcon);
+      jest.runAllTimers();
+    });
+
+    await waitFor(() => {
+      const tooltip = screen.getByText(/Please click on the step to configure branches for it./);
+      expect(tooltip).toBeInTheDocument();
+    });
+
+    spy.mockReset();
+  });
 });
diff --git a/src/components/AppendStepButton.tsx b/src/components/AppendStepButton.tsx
@@ -1,18 +1,17 @@
-import { BranchBuilder } from './BranchBuilder';
-import { MiniCatalog } from './MiniCatalog';
-import { StepsService, ValidationService } from '@kaoto/services';
+import { StepsService, ValidationService, VisualizationService } from '@kaoto/services';
 import { useIntegrationJsonStore, useSettingsStore } from '@kaoto/store';
 import { IStepProps } from '@kaoto/types';
 import { Popover, Tooltip } from '@patternfly/react-core';
 import { PlusIcon } from '@patternfly/react-icons';
 import { FunctionComponent, useEffect, useState } from 'react';
+import { BranchBuilder } from './BranchBuilder';
+import { MiniCatalog } from './MiniCatalog';
 
 interface IAddStepButton {
   handleAddBranch: () => void;
   handleSelectStep: (selectedStep: IStepProps) => void;
   layout: string;
   step: IStepProps;
-  showBranchesTab: boolean;
   showStepsTab: boolean;
   supportsBranching: boolean;
 }
@@ -22,7 +21,6 @@ export const AppendStepButton: FunctionComponent<IAddStepButton> = ({
   handleSelectStep,
   layout,
   step,
-  showBranchesTab,
   showStepsTab,
   supportsBranching,
 }) => {
@@ -31,12 +29,20 @@ export const AppendStepButton: FunctionComponent<IAddStepButton> = ({
   const [hasCustomStepExtension, setHasCustomStepExtension] = useState(
     StepsService.hasCustomStepExtension(step, views)
   );
+  const [disableBranchesTab, setDisableBranchesTab] = useState(false);
   const [disableBranchesTabMsg, setDisableBranchesTabMsg] = useState('');
+  const [tooltipText, setTooltipText] = useState('');
+  const [disableButton, setDisableButton] = useState(false);
 
   useEffect(() => {
     setHasCustomStepExtension(StepsService.hasCustomStepExtension(step, views));
   }, [step, views]);
 
+  useEffect(() => {
+    const showBranchesTab = VisualizationService.showBranchesTab(step);
+    setDisableBranchesTab(hasCustomStepExtension || !showBranchesTab || !supportsBranching);
+  }, [step, hasCustomStepExtension, supportsBranching]);
+
   useEffect(() => {
     if (hasCustomStepExtension) {
       setDisableBranchesTabMsg('Please click on the step to configure branches for it.');
@@ -50,15 +56,23 @@ export const AppendStepButton: FunctionComponent<IAddStepButton> = ({
         step.branches?.length
       )
     );
-  }, [hasCustomStepExtension, step, supportsBranching, views]);
+  }, [hasCustomStepExtension, step, supportsBranching]);
+
+  useEffect(() => {
+    setDisableButton(!showStepsTab && disableBranchesTab);
+  }, [showStepsTab, disableBranchesTab]);
+
+  useEffect(() => {
+    setTooltipText(ValidationService.getPlusButtonTooltipMsg(!disableBranchesTab, showStepsTab));
+  }, [disableBranchesTab, showStepsTab]);
 
   return (
     <Popover
       id="popover-append-step"
       aria-label="Add a step or branch"
       bodyContent={
         <MiniCatalog
-          disableBranchesTab={hasCustomStepExtension || !showBranchesTab}
+          disableBranchesTab={disableBranchesTab}
           disableBranchesTabMsg={disableBranchesTabMsg}
           disableStepsTab={!showStepsTab}
           disableStepsTabMsg="You can't add a step between a step and a branch."
@@ -74,22 +88,23 @@ export const AppendStepButton: FunctionComponent<IAddStepButton> = ({
       }
       className="miniCatalog__popover"
       data-testid="miniCatalog__popover"
-      enableFlip={true}
+      enableFlip
       flipBehavior={['top-start', 'left-start']}
       hasAutoWidth
-      hideOnOutsideClick={true}
+      hideOnOutsideClick
       position="right-start"
       showClose={false}
     >
       <Tooltip
-        content={ValidationService.getPlusButtonTooltipMsg(showBranchesTab, showStepsTab)}
+        content={tooltipText}
         position={layout === 'LR' ? 'top' : 'right'}
       >
         <button
-          className={`${
-            layout === 'LR' ? 'stepNode__Add' : 'stepNode__Add--vertical'
-          } plusButton nodrag`}
+          className={`${layout === 'LR' ? 'stepNode__Add' : 'stepNode__Add--vertical'
+            } plusButton nodrag`}
           data-testid="stepNode__appendStep-btn"
+          disabled={disableButton}
+          aria-disabled={disableButton}
         >
           <PlusIcon />
         </button>

diff --git a/src/components/MiniCatalog.test.tsx b/src/components/MiniCatalog.test.tsx
@@ -1,27 +1,23 @@
+import { act, render, screen, waitFor } from '@testing-library/react';
 import { AlertProvider } from '../layout';
 import { MiniCatalog } from './MiniCatalog';
-import { screen } from '@testing-library/dom';
-import { render } from '@testing-library/react';
 
 describe('MiniCatalog.tsx', () => {
-  test('component renders correctly', () => {
-    render(
-      <AlertProvider>
-        <MiniCatalog />
-      </AlertProvider>
-    );
+  test('component renders correctly', async () => {
+    act(() => {
+      render(
+        <AlertProvider>
+          <MiniCatalog />
+        </AlertProvider>
+      );
+    });
 
-    const element = screen.getByTestId('miniCatalog');
-    expect(element).toBeInTheDocument();
-  });
-  test('component renders correctly', () => {
-    render(
-      <AlertProvider>
-        <MiniCatalog />
-      </AlertProvider>
-    );
+    await waitFor(() => {
+      const element = screen.getByTestId('miniCatalog');
+      expect(element).toBeInTheDocument();
 
-    const element = screen.getByText('start');
-    expect(element).toBeInTheDocument();
+      const startButton = screen.getByText('start');
+      expect(startButton).toBeInTheDocument();
+    });
   });
 });
diff --git a/src/components/PlusButtonEdge.tsx b/src/components/PlusButtonEdge.tsx
@@ -45,7 +45,7 @@ const PlusButtonEdge = ({
   const nestedStepsStore = useNestedStepsStore();
   const visualizationStore = useVisualizationStore();
   const stepsService = new StepsService(integrationJsonStore, nestedStepsStore, visualizationStore);
-  const showBranchesTab = VisualizationService.showBranchesTab(sourceNode?.data);
+  const showBranchesTab = VisualizationService.showBranchesTab(sourceNode?.data.step);
   const showStepsTab = VisualizationService.showStepsTab(sourceNode?.data);
 
   const [edgePath, edgeCenterX, edgeCenterY] = getBezierPath({

diff --git a/src/components/Visualization.test.tsx b/src/components/Visualization.test.tsx
@@ -1,5 +1,6 @@
 import { IIntegrationJsonStore, RFState, useIntegrationJsonStore, useVisualizationStore } from '@kaoto/store';
 import { fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { act } from 'react-dom/test-utils';
 import { AlertProvider } from '../layout';
 import { integrationJSONStub, stepsStub } from '../__mocks__/steps';
 import { Visualization } from './Visualization';
@@ -33,14 +34,19 @@ beforeAll(() => {
 });
 
 describe('Visualization.tsx', () => {
-  test('component renders correctly', () => {
-    render(
-      <AlertProvider>
-        <Visualization />
-      </AlertProvider>
-    );
-    const element = screen.getByTestId('react-flow-wrapper');
-    expect(element).toBeInTheDocument();
+  test('component renders correctly', async () => {
+    act(() => {
+      render(
+        <AlertProvider>
+          <Visualization />
+        </AlertProvider>
+      );
+    });
+
+    await waitFor(() => {
+      const element = screen.getByTestId('react-flow-wrapper');
+      expect(element).toBeInTheDocument();
+    });
   });
 
   test('should expands the details panel upon clicking on a step', async () => {

diff --git a/src/components/VisualizationStep.tsx b/src/components/VisualizationStep.tsx
@@ -26,7 +26,7 @@ const VisualizationStep = ({ data }: NodeProps<IVizStepNodeData>) => {
   const integrationJsonStore = useIntegrationJsonStore();
   const visualizationService = new VisualizationService(integrationJsonStore, visualizationStore);
   const stepsService = new StepsService(integrationJsonStore, nestedStepsStore, visualizationStore);
-  const showBranchesTab = VisualizationService.showBranchesTab(data);
+  const showBranchesTab = VisualizationService.showBranchesTab(data.step);
   const showStepsTab = VisualizationService.showStepsTab(data);
   const supportsBranching = StepsService.supportsBranching(data.step);
 
@@ -187,7 +187,6 @@ const VisualizationStep = ({ data }: NodeProps<IVizStepNodeData>) => {
               handleSelectStep={onMiniCatalogClickAppend}
               layout={visualizationStore.layout}
               step={data.step}
-              showBranchesTab={showBranchesTab}
               showStepsTab={showStepsTab}
               supportsBranching={supportsBranching}
             />

diff --git a/src/services/validationService.test.ts b/src/services/validationService.test.ts
@@ -108,7 +108,7 @@ describe('validationService', () => {
     expect(ValidationService.getPlusButtonTooltipMsg(true, true)).toBe('Add a step or branch');
     expect(ValidationService.getPlusButtonTooltipMsg(true, false)).toBe('Add a branch');
     expect(ValidationService.getPlusButtonTooltipMsg(false, true)).toBe('Add a step');
-    expect(ValidationService.getPlusButtonTooltipMsg(false, false)).toBe('');
+    expect(ValidationService.getPlusButtonTooltipMsg(false, false)).toBe('Please click on the step to configure branches for it.');
   });
 
   it('prependableStepTypes(): should return a comma-separated string of step types that can be prepended to a step', () => {

diff --git a/src/services/validationService.ts b/src/services/validationService.ts
@@ -116,7 +116,7 @@ export class ValidationService {
     } else if (showStepsTab) {
       return 'Add a step';
     } else {
-      return '';
+      return 'Please click on the step to configure branches for it.';
     }
   }
 

diff --git a/src/services/visualizationService.test.ts b/src/services/visualizationService.test.ts
@@ -607,37 +607,33 @@ describe('visualizationService', () => {
   });
 
   it('showBranchesTab(): given node data, should determine whether to show the branches tab in mini catalog', () => {
-    const step: IVizStepNodeData = {
-      label: '',
-      step: {} as IStepProps,
-    };
+    const step = {} as IStepProps;
 
     expect(VisualizationService.showBranchesTab(step)).toBeFalsy();
     // has branches but not branch support
     expect(
       VisualizationService.showBranchesTab({
         ...step,
-        step: { ...step.step, branches: [] },
+        branches: [],
       })
     ).toBeFalsy();
 
     expect(
       VisualizationService.showBranchesTab({
         ...step,
-        step: { ...step.step, branches: [], minBranches: 0, maxBranches: -1 },
+        branches: [],
+        minBranches: 0,
+        maxBranches: -1,
       })
     ).toBeTruthy();
 
     // if step has maximum number of branches already
     expect(
       VisualizationService.showBranchesTab({
         ...step,
-        step: {
-          ...step.step,
-          branches: [{}, {}] as IStepPropsBranch[],
-          minBranches: 0,
-          maxBranches: 2,
-        },
+        branches: [{}, {}] as IStepPropsBranch[],
+        minBranches: 0,
+        maxBranches: 2,
       })
     ).toBeFalsy();
   });

diff --git a/src/services/visualizationService.ts b/src/services/visualizationService.ts
@@ -626,10 +626,10 @@ export class VisualizationService {
    * Determines whether to show the Branches tab in the mini catalog
    * @param nodeData
    */
-  static showBranchesTab(nodeData: IVizStepNodeData): boolean {
+  static showBranchesTab(step: IStepProps): boolean {
     return (
-      StepsService.supportsBranching(nodeData.step) &&
-      nodeData.step.branches?.length !== nodeData.step.maxBranches
+      StepsService.supportsBranching(step) &&
+      step.branches?.length !== step.maxBranches
     );
   }