fix(Wizard): onStepChange - skip isDisabled & isHidden

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');
 });