fix(Wizard): onStepChange - skip isDisabled & isHidden

Author: jpuzz0

packages/react-core/src/components/Wizard/Wizard.tsx

@@ -11,7 +11,7 @@ import {
   WizardNavType,
   WizardStepChangeScope
 } from './types';
-import { buildSteps } from './utils';
+import { buildSteps, isStepEnabled } from './utils';
 import { useWizardContext, WizardContextProvider } from './WizardContext';
 import { WizardToggle } from './WizardToggle';
 import { WizardNavInternal } from './WizardNavInternal';
@@ -23,7 +23,7 @@ import { WizardNavInternal } from './WizardNavInternal';
 
 export interface WizardProps extends React.HTMLProps<HTMLDivElement> {
   /** Step components */
-  children: React.ReactNode | React.ReactNode[];
+  children: React.ReactNode;
   /** Wizard header */
   header?: React.ReactNode;
   /** Wizard footer */
@@ -86,35 +86,23 @@ export const Wizard = ({
   }, [startIndex]);
 
   const goToNextStep = (event: React.MouseEvent<HTMLButtonElement>, steps: WizardStepType[] = initialSteps) => {
-    const newStep = steps.find(
-      (step) => step.index > activeStepIndex && !step.isHidden && !step.isDisabled && !isWizardParentStep(step)
-    );
+    const newStep = steps.find((step) => step.index > activeStepIndex && isStepEnabled(steps, step));
 
     if (activeStepIndex >= steps.length || !newStep?.index) {
       return onSave ? onSave(event) : onClose?.(event);
     }
 
-    const currStep = isWizardParentStep(steps[activeStepIndex]) ? steps[activeStepIndex + 1] : steps[activeStepIndex];
-    const prevStep = steps[activeStepIndex - 1];
-
     setActiveStepIndex(newStep?.index);
-    onStepChange?.(event, currStep, prevStep, WizardStepChangeScope.Next);
+    onStepChange?.(event, newStep, steps[activeStepIndex - 1], WizardStepChangeScope.Next);
   };
 
   const goToPrevStep = (event: React.MouseEvent<HTMLButtonElement>, steps: WizardStepType[] = initialSteps) => {
     const newStep = [...steps]
       .reverse()
-      .find(
-        (step: WizardStepType) =>
-          step.index < activeStepIndex && !step.isHidden && !step.isDisabled && !isWizardParentStep(step)
-      );
-    const currStep = isWizardParentStep(steps[activeStepIndex - 2])
-      ? steps[activeStepIndex - 3]
-      : steps[activeStepIndex - 2];
-    const prevStep = steps[activeStepIndex - 1];
+      .find((step: WizardStepType) => step.index < activeStepIndex && isStepEnabled(steps, step));
 
     setActiveStepIndex(newStep?.index);
-    onStepChange?.(event, currStep, prevStep, WizardStepChangeScope.Back);
+    onStepChange?.(event, newStep, steps[activeStepIndex - 1], WizardStepChangeScope.Back);
   };
 
   const goToStepByIndex = (

packages/react-core/src/components/Wizard/WizardBody.tsx

@@ -12,7 +12,7 @@ import { getResizeObserver } from '../../helpers/resizeObserver';
 
 export interface WizardBodyProps {
   /** Anything that can be rendered in the Wizard body */
-  children: React.ReactNode | React.ReactNode[];
+  children: React.ReactNode;
   /** Flag to remove the default body padding */
   hasNoPadding?: boolean;
   /** Adds an accessible name to the wrapper element when the content overflows and renders
@@ -67,7 +67,7 @@ export const WizardBody = ({
     return () => {
       observer();
     };
-  }, []);
+  }, [previousWidth]);
 
   return (
     <WrapperComponent

packages/react-core/src/components/Wizard/WizardNavInternal.tsx

@@ -85,7 +85,7 @@ export const WizardNavInternal = ({
                     id={subStep.id}
                     content={subStep.name}
                     isCurrent={activeStep?.id === subStep.id}
-                    isDisabled={isSubStepDisabled}
+                    isDisabled={isSubStepDisabled || isStepDisabled}
                     isVisited={subStep.isVisited}
                     stepIndex={subStep.index}
                     onClick={() => goToStepByIndex(subStep.index)}
@@ -109,7 +109,7 @@ export const WizardNavInternal = ({
                   content={step.name}
                   isExpandable={step.isExpandable}
                   isCurrent={hasActiveChild}
-                  isDisabled={!hasEnabledChildren}
+                  isDisabled={!hasEnabledChildren || isStepDisabled}
                   isVisited={step.isVisited}
                   stepIndex={firstSubStepIndex}
                   onClick={() => goToStepByIndex(firstSubStepIndex)}

packages/react-core/src/components/Wizard/WizardToggle.tsx

@@ -71,7 +71,7 @@ export const WizardToggle = ({
 
     return (
       <React.Fragment key={step.id}>
-        {activeStep?.name === step.name &&
+        {activeStep?.id === step.id &&
           (body || body === undefined ? <WizardBody {...body}>{children}</WizardBody> : children)}
 
         <div key={step.id} style={{ display: 'none' }}>

packages/react-core/src/components/Wizard/__tests__/Wizard.test.tsx

@@ -3,7 +3,7 @@ import React from 'react';
 import { render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 
-import { Wizard, WizardFooterProps, WizardStep, WizardNavProps } from '../';
+import { Wizard, WizardFooterProps, WizardStep, WizardNavProps, WizardStepChangeScope } from '../';
 
 test('renders step when child is of type WizardStep', () => {
   render(
@@ -100,7 +100,6 @@ test('renders default nav with custom props', () => {
 });
 
 test('renders nav aria label', () => {
-
   render(
     <Wizard navAriaLabel="custom nav aria-label">
       <WizardStep id="test-step" name="Test step" />
@@ -174,45 +173,55 @@ test(`can customize the wizard's height and width`, () => {
   expect(wizard).toHaveStyle('width: 500px');
 });
 
-test('calls onNavByIndex on nav item click', async () => {
+test('calls onStepChange on nav item click', async () => {
   const user = userEvent.setup();
-  const onNavByIndex = jest.fn();
+  const onStepChange = jest.fn();
 
   render(
-    <Wizard onStepChange={onNavByIndex}>
+    <Wizard onStepChange={onStepChange}>
       <WizardStep id="step-1" name="Test step 1" />
       <WizardStep id="step-2" name="Test step 2" />
     </Wizard>
   );
 
   await user.click(screen.getByRole('button', { name: 'Test step 2' }));
-  expect(onNavByIndex).toHaveBeenCalled();
+  expect(onStepChange).toHaveBeenCalledWith(
+    null,
+    expect.objectContaining({ id: 'step-2' }),
+    expect.objectContaining({ id: 'step-1' }),
+    WizardStepChangeScope.Nav
+  );
 });
 
-test('calls onNext and not onSave on next button click when not on the last step', async () => {
+test('calls onStepChange and not onSave on next button click when not on the last step', async () => {
   const user = userEvent.setup();
-  const onNext = jest.fn();
+  const onStepChange = jest.fn();
   const onSave = jest.fn();
 
   render(
-    <Wizard onStepChange={onNext} onSave={onSave}>
+    <Wizard onStepChange={onStepChange} onSave={onSave}>
       <WizardStep id="step-1" name="Test step 1" />
       <WizardStep id="step-2" name="Test step 2" />
     </Wizard>
   );
 
   await user.click(screen.getByRole('button', { name: 'Next' }));
 
-  expect(onNext).toHaveBeenCalled();
+  expect(onStepChange).toHaveBeenCalledWith(
+    null,
+    expect.objectContaining({ id: 'step-2' }),
+    expect.objectContaining({ id: 'step-1' }),
+    WizardStepChangeScope.Next
+  );
   expect(onSave).not.toHaveBeenCalled();
 });
 
-test('calls onBack on back button click', async () => {
+test('calls onStepChange on back button click', async () => {
   const user = userEvent.setup();
-  const onBack = jest.fn();
+  const onStepChange = jest.fn();
 
   render(
-    <Wizard onStepChange={onBack}>
+    <Wizard onStepChange={onStepChange}>
       <WizardStep id="step-1" name="Test step 1" />
       <WizardStep id="step-2" name="Test step 2" />
     </Wizard>
@@ -221,7 +230,12 @@ test('calls onBack on back button click', async () => {
   await user.click(screen.getByRole('button', { name: 'Test step 2' }));
   await user.click(screen.getByRole('button', { name: 'Back' }));
 
-  expect(onBack).toHaveBeenCalled();
+  expect(onStepChange).toHaveBeenCalledWith(
+    null,
+    expect.objectContaining({ id: 'step-1' }),
+    expect.objectContaining({ id: 'step-2' }),
+    WizardStepChangeScope.Back
+  );
 });
 
 test('calls onSave and not onClose on next button click when on the last step', async () => {
@@ -445,17 +459,153 @@ test('incrementally shows/hides steps based on the activeStep when isProgressive
   ).toBeNull();
 });
 
-test('parent step can be non-collapsible by setting isCollapsible to false', () => {
+test('parent step can be expandable by setting isExpandable to true', () => {
   render(
     <Wizard>
       <WizardStep
         id="step-1"
         name="Test step 1"
-        isCollapsible={false}
+        isExpandable
         steps={[<WizardStep id="sub-step-1" name="Sub step 1" />]}
       />
     </Wizard>
   );
 
-  expect(screen.queryByLabelText('step icon', { exact: false })).toBeNull();
+  expect(screen.getByLabelText('step icon', { exact: false })).toBeVisible();
+});
+
+test('child steps are disabled when parent is disabled', () => {
+  render(
+    <Wizard>
+      <WizardStep
+        id="step-1"
+        name="Test step 1"
+        isDisabled
+        steps={[<WizardStep id="sub-step-1" name="Sub step 1" />]}
+      />
+    </Wizard>
+  );
+
+  expect(
+    screen.getByRole('button', {
+      name: 'Test step 1'
+    })
+  ).toBeDisabled();
+  expect(
+    screen.getByRole('button', {
+      name: 'Sub step 1'
+    })
+  ).toBeDisabled();
+});
+
+test('child steps are hidden when parent is hidden', () => {
+  render(
+    <Wizard>
+      <WizardStep id="step-1" name="Test step 1" isHidden steps={[<WizardStep id="sub-step-1" name="Sub step 1" />]} />
+    </Wizard>
+  );
+
+  expect(
+    screen.queryByRole('button', {
+      name: 'Test step 1'
+    })
+  ).toBeNull();
+  expect(
+    screen.queryByRole('button', {
+      name: 'Sub step 1'
+    })
+  ).toBeNull();
+});
+
+test('onStepChange skips over disabled or hidden steps and substeps', async () => {
+  const user = userEvent.setup();
+  const onStepChange = jest.fn();
+
+  render(
+    <Wizard onStepChange={onStepChange}>
+      <WizardStep id="step-1" name="Test step 1" />
+      <WizardStep id="step-2" name="Test step 2" steps={[<WizardStep id="step2-sub1" name="Test Substep 1" />]} />
+      <WizardStep id="step-3" name="Test step 3" isDisabled />
+      <WizardStep
+        id="step-4"
+        name="Test step 4"
+        isDisabled
+        steps={[<WizardStep id="step4-sub1" name="Test Substep 1" />]}
+      />
+      <WizardStep
+        id="step-5"
+        name="Test step 4"
+        steps={[
+          <WizardStep id="step5-sub1" name="Test Substep 1" isDisabled />,
+          <WizardStep id="step5-sub2" name="Test Substep 2" />
+        ]}
+      />
+      <WizardStep id="step-6" name="Test step 6" isHidden />
+      <WizardStep
+        id="step-7"
+        name="Test step 7"
+        isHidden
+        steps={[<WizardStep id="step7-sub1" name="Test Substep 1" />]}
+      />
+      <WizardStep
+        id="step-8"
+        name="Test step 8"
+        steps={[
+          <WizardStep id="step8-sub1" name="Test Substep 1" isHidden />,
+          <WizardStep id="step8-sub2" name="Test Substep 2" />
+        ]}
+      />
+    </Wizard>
+  );
+
+  const nextButton = screen.getByRole('button', { name: 'Next' });
+  const backButton = screen.getByRole('button', { name: 'Back' });
+
+  await user.click(nextButton);
+  expect(onStepChange).toHaveBeenCalledWith(
+    null,
+    expect.objectContaining({ id: 'step2-sub1' }),
+    expect.objectContaining({ id: 'step-1' }),
+    WizardStepChangeScope.Next
+  );
+
+  await user.click(nextButton);
+  expect(onStepChange).toHaveBeenCalledWith(
+    null,
+    expect.objectContaining({ id: 'step5-sub2' }),
+    expect.objectContaining({ id: 'step2-sub1' }),
+    WizardStepChangeScope.Next
+  );
+
+  await user.click(nextButton);
+  expect(onStepChange).toHaveBeenCalledWith(
+    null,
+    expect.objectContaining({ id: 'step8-sub2' }),
+    expect.objectContaining({ id: 'step5-sub2' }),
+    WizardStepChangeScope.Next
+  );
+
+  await user.click(backButton);
+  expect(onStepChange).toHaveBeenCalledWith(
+    null,
+    expect.objectContaining({ id: 'step5-sub2' }),
+    expect.objectContaining({ id: 'step8-sub2' }),
+    WizardStepChangeScope.Back
+  );
+
+  await user.click(backButton);
+  expect(onStepChange).toHaveBeenCalledWith(
+    null,
+    expect.objectContaining({ id: 'step2-sub1' }),
+    expect.objectContaining({ id: 'step5-sub2' }),
+    WizardStepChangeScope.Back
+  );
+
+  await user.click(backButton);
+  expect(onStepChange).toHaveBeenCalledWith(
+    null,
+    expect.objectContaining({ id: 'step-1' }),
+    expect.objectContaining({ id: 'step2-sub1' }),
+    WizardStepChangeScope.Back
+  );
 });

packages/react-core/src/components/Wizard/__tests__/WizardBody.test.tsx

@@ -10,7 +10,7 @@ test('renders children without additional props', () => {
   expect(container).not.toHaveAttribute('aria-labelledby');
 });
 
-test('has no padding className when hasNoBodyPadding is not specified', () => {
+test('has no padding className when hasNoPadding is not specified', () => {
   render(<WizardBody>content</WizardBody>);
   expect(screen.getByText('content')).not.toHaveClass('pf-m-no-padding');
 });