From 57e59ef4a2e1f2f6d72acdb4400fc4a32b54bbdb Mon Sep 17 00:00:00 2001 From: Jeff Puzzo Date: Tue, 6 Sep 2022 10:22:26 -0400 Subject: [PATCH 1/2] feat(Wizard - next): supporting component unit tests --- package.json | 1 + .../src/next/components/Wizard/Wizard.tsx | 28 ++---- .../src/next/components/Wizard/WizardBody.tsx | 2 +- .../next/components/Wizard/WizardContext.tsx | 32 ++++++- .../next/components/Wizard/WizardNavItem.tsx | 5 +- .../src/next/components/Wizard/WizardStep.tsx | 8 +- .../next/components/Wizard/WizardToggle.tsx | 6 +- .../Wizard/__tests__/Wizard.test.tsx | 40 ++++++++ .../Wizard/__tests__/WizardBody.test.tsx | 43 +++++++++ .../Wizard/__tests__/WizardFooter.test.tsx | 95 +++++++++++++++++++ .../Wizard/__tests__/WizardStep.test.tsx | 35 +++++++ .../Wizard/__tests__/WizardToggle.test.tsx | 80 ++++++++++++++++ .../Wizard/__tests__/utils.test.tsx | 66 +++++++++++++ .../next/components/Wizard/examples/Wizard.md | 2 - .../Wizard/examples/WizardKitchenSink.tsx | 14 +-- .../hooks/__tests__/useWizardFooter.test.tsx | 51 ++++++++++ .../Wizard/hooks/useWizardFooter.tsx | 4 +- .../src/next/components/Wizard/types.tsx | 6 ++ .../src/next/components/Wizard/utils.ts | 44 +++++---- yarn.lock | 15 +++ 20 files changed, 508 insertions(+), 69 deletions(-) create mode 100644 packages/react-core/src/next/components/Wizard/__tests__/WizardBody.test.tsx create mode 100644 packages/react-core/src/next/components/Wizard/__tests__/WizardFooter.test.tsx create mode 100644 packages/react-core/src/next/components/Wizard/__tests__/WizardStep.test.tsx create mode 100644 packages/react-core/src/next/components/Wizard/__tests__/WizardToggle.test.tsx create mode 100644 packages/react-core/src/next/components/Wizard/__tests__/utils.test.tsx create mode 100644 packages/react-core/src/next/components/Wizard/hooks/__tests__/useWizardFooter.test.tsx diff --git a/package.json b/package.json index 6066f2dfb0a..e19af5bd6f3 100644 --- a/package.json +++ b/package.json @@ -35,6 +35,7 @@ "@babel/preset-react": "^7.0.0", "@babel/preset-typescript": "^7.9.0", "@octokit/rest": "^16.43.2", + "@testing-library/react-hooks": "^8.0.1", "@testing-library/jest-dom": "^5.16.2", "@testing-library/react": "^12.1.5", "@testing-library/user-event": "14.4.3", diff --git a/packages/react-core/src/next/components/Wizard/Wizard.tsx b/packages/react-core/src/next/components/Wizard/Wizard.tsx index 040a8061335..5c8b1733c8c 100644 --- a/packages/react-core/src/next/components/Wizard/Wizard.tsx +++ b/packages/react-core/src/next/components/Wizard/Wizard.tsx @@ -6,7 +6,6 @@ import styles from '@patternfly/react-styles/css/components/Wizard/wizard'; import { DefaultWizardFooterProps, DefaultWizardNavProps, - isCustomWizardFooter, isWizardParentStep, WizardNavStepFunction, CustomWizardNavFunction @@ -14,7 +13,6 @@ import { import { buildSteps, normalizeNavStep } from './utils'; import { useWizardContext, WizardContextProvider } from './WizardContext'; import { WizardStepProps } from './WizardStep'; -import { WizardFooter } from './WizardFooter'; import { WizardToggle } from './WizardToggle'; /** @@ -39,6 +37,8 @@ export interface WizardProps extends React.HTMLProps { width?: number | string; /** Custom height of the wizard */ height?: number | string; + /** Flag to unmount inactive steps instead of hiding. Defaults to true */ + unmountInactiveSteps?: boolean; /** Callback function when a step in the nav is clicked */ onNavByIndex?: WizardNavStepFunction; /** Callback function after next button is clicked */ @@ -130,7 +130,7 @@ export const Wizard = (props: WizardProps) => { { goToStepByName={goToStepByName} goToStepByIndex={goToStepByIndex} > - - {children} - + {children} ); }; // eslint-disable-next-line patternfly-react/no-anonymous-functions -const WizardInternal = ({ height, width, className, header, footer, nav, ...divProps }: WizardProps) => { - const { activeStep, steps, footer: customFooter, onNext, onBack, onClose, goToStepByIndex } = useWizardContext(); - - const wizardFooter = customFooter || ( - - ); +const WizardInternal = ({ height, width, className, header, nav, unmountInactiveSteps, ...divProps }: WizardProps) => { + const { activeStep, steps, footer, goToStepByIndex } = useWizardContext(); return (
); diff --git a/packages/react-core/src/next/components/Wizard/WizardBody.tsx b/packages/react-core/src/next/components/Wizard/WizardBody.tsx index 51cd195facc..c9d4e3c40bb 100644 --- a/packages/react-core/src/next/components/Wizard/WizardBody.tsx +++ b/packages/react-core/src/next/components/Wizard/WizardBody.tsx @@ -8,7 +8,7 @@ import { css } from '@patternfly/react-styles'; */ export interface WizardBodyProps { - children?: React.ReactNode | React.ReactNode[]; + children: React.ReactNode | React.ReactNode[]; /** Set to true to remove the default body padding */ hasNoBodyPadding?: boolean; /** An aria-label to use for the wrapper element */ diff --git a/packages/react-core/src/next/components/Wizard/WizardContext.tsx b/packages/react-core/src/next/components/Wizard/WizardContext.tsx index 8859d1b5879..7e93bd8914d 100644 --- a/packages/react-core/src/next/components/Wizard/WizardContext.tsx +++ b/packages/react-core/src/next/components/Wizard/WizardContext.tsx @@ -1,6 +1,11 @@ import React from 'react'; -import { WizardControlStep } from './types'; + +import { css } from '@patternfly/react-styles'; +import styles from '@patternfly/react-styles/css/components/Wizard/wizard'; + +import { DefaultWizardFooterProps, isCustomWizardFooter, WizardControlStep } from './types'; import { getActiveStep } from './utils'; +import { WizardFooter } from './WizardFooter'; export interface WizardContextProps { /** List of steps */ @@ -22,7 +27,7 @@ export interface WizardContextProps { /** Navigate to step by index */ goToStepByIndex: (index: number) => void; /** Update the footer with any react element */ - setFooter: (footer: React.ReactElement) => void; + setFooter: (footer: DefaultWizardFooterProps | React.ReactElement) => void; } export const WizardContext = React.createContext({} as WizardContextProps); @@ -30,7 +35,7 @@ export const WizardContext = React.createContext({} as WizardContextProps); interface WizardContextRenderProps { steps: WizardControlStep[]; activeStep: WizardControlStep; - footer: React.ReactElement; + footer: DefaultWizardFooterProps | React.ReactElement; onNext(): void; onBack(): void; onClose(): void; @@ -39,7 +44,7 @@ interface WizardContextRenderProps { export interface WizardContextProviderProps { steps: WizardControlStep[]; currentStepIndex: number; - footer: React.ReactElement; + footer: DefaultWizardFooterProps | React.ReactElement; children: React.ReactElement | ((props: WizardContextRenderProps) => React.ReactElement); onNext(): void; onBack(): void; @@ -66,6 +71,23 @@ export const WizardContextProvider: React.FunctionComponent + isCustomWizardFooter(footer) ? ( +
{footer}
+ ) : ( + + ), + [activeStep, footer, onBack, onClose, onNext, steps] + ); + // When the active step changes and the newly active step isn't visited, set the visited flag to true. React.useEffect(() => { if (activeStep && !activeStep?.visited) { @@ -86,7 +108,7 @@ export const WizardContextProvider: React.FunctionComponent = ({ {...rest} {...(navItemComponent === 'a' ? { ...linkProps } : { ...btnProps })} {...(id && { id: id.toString() })} - onClick={() => (isExpandable ? setIsExpanded(!isExpanded || isCurrent) : onNavItemClick(step))} + onClick={e => { + e.preventDefault(); + isExpandable ? setIsExpanded(!isExpanded || isCurrent) : onNavItemClick(step); + }} className={css( styles.wizardNavLink, isCurrent && styles.modifiers.current, diff --git a/packages/react-core/src/next/components/Wizard/WizardStep.tsx b/packages/react-core/src/next/components/Wizard/WizardStep.tsx index 4a0f7a65564..64b32cde34f 100644 --- a/packages/react-core/src/next/components/Wizard/WizardStep.tsx +++ b/packages/react-core/src/next/components/Wizard/WizardStep.tsx @@ -1,6 +1,6 @@ import React from 'react'; -import { WizardControlStep } from './types'; +import { WizardBasicStep } from './types'; import { WizardBody, WizardBodyProps } from './WizardBody'; /** @@ -8,16 +8,16 @@ import { WizardBody, WizardBodyProps } from './WizardBody'; * Also acts as a wrapper for content, with an optional inclusion of WizardBody. */ -export interface WizardStepProps extends Omit { +export interface WizardStepProps extends Omit { /** Optional for when the step is used as a parent to sub-steps */ children?: React.ReactNode; /** Props for WizardBody that wraps content by default. Can be set to null for exclusion of WizardBody. */ - body?: WizardBodyProps | null; + body?: Omit | null; /** Optional list of sub-steps */ steps?: React.ReactElement[]; } export const WizardStep = ({ body, children }: WizardStepProps) => - body === undefined ? {children} : <>{children}; + body || body === undefined ? {children} : <>{children}; WizardStep.displayName = 'WizardStep'; diff --git a/packages/react-core/src/next/components/Wizard/WizardToggle.tsx b/packages/react-core/src/next/components/Wizard/WizardToggle.tsx index 7e67983a840..03552bf381f 100644 --- a/packages/react-core/src/next/components/Wizard/WizardToggle.tsx +++ b/packages/react-core/src/next/components/Wizard/WizardToggle.tsx @@ -28,11 +28,11 @@ export interface WizardToggleProps { activeStep: WizardControlStep; /** The WizardFooter */ footer: React.ReactElement; - /** Custom WizardNav or callback used to create a default WizardNav */ - nav: DefaultWizardNavProps | CustomWizardNavFunction; /** Navigate using the step index */ goToStepByIndex: (index: number) => void; - /** The button's aria-label */ + /** Custom WizardNav or callback used to create a default WizardNav */ + nav?: DefaultWizardNavProps | CustomWizardNavFunction; + /** The expandable dropdown button's aria-label */ 'aria-label'?: string; /** Flag to unmount inactive steps instead of hiding. Defaults to true */ unmountInactiveSteps?: boolean; diff --git a/packages/react-core/src/next/components/Wizard/__tests__/Wizard.test.tsx b/packages/react-core/src/next/components/Wizard/__tests__/Wizard.test.tsx index e0c36ef688b..0ca02562928 100644 --- a/packages/react-core/src/next/components/Wizard/__tests__/Wizard.test.tsx +++ b/packages/react-core/src/next/components/Wizard/__tests__/Wizard.test.tsx @@ -301,4 +301,44 @@ describe('Wizard', () => { await user.click(screen.getByRole('button', { name: 'Cancel' })); expect(onClose).toHaveBeenCalled(); }); + + it('unmounts inactive steps by default', async () => { + const user = userEvent.setup(); + + render( + + + Step 1 content + + + Step 2 content + + + ); + + await user.click(screen.getByRole('button', { name: 'Test step 2' })); + + expect(screen.queryByText('Step 1 content')).toBeNull(); + expect(screen.getByText('Step 2 content')).toBeVisible(); + }); + + it('keeps inactive steps mounted when unmountInactiveSteps is enabled', async () => { + const user = userEvent.setup(); + + render( + + + Step 1 content + + + Step 2 content + + + ); + + await user.click(screen.getByRole('button', { name: 'Test step 2' })); + + expect(screen.getByText('Step 1 content')).toBeInTheDocument(); + expect(screen.getByText('Step 2 content')).toBeVisible(); + }); }); diff --git a/packages/react-core/src/next/components/Wizard/__tests__/WizardBody.test.tsx b/packages/react-core/src/next/components/Wizard/__tests__/WizardBody.test.tsx new file mode 100644 index 00000000000..920db3a79a9 --- /dev/null +++ b/packages/react-core/src/next/components/Wizard/__tests__/WizardBody.test.tsx @@ -0,0 +1,43 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import { WizardBody } from '../WizardBody'; + +describe('WizardBody', () => { + it('renders children without additional props', () => { + const { container } = render(content); + + expect(container).toHaveTextContent('content'); + expect(container).not.toHaveAttribute('aria-label'); + expect(container).not.toHaveAttribute('aria-labelledby'); + }); + + it('has no padding className when hasNoBodyPadding is not specified', () => { + render(content); + expect(screen.getByText('content')).not.toHaveClass('pf-m-no-padding'); + }); + + it('has padding className when hasNoBodyPadding is specified', () => { + render(content); + expect(screen.getByText('content')).toHaveClass('pf-m-no-padding'); + }); + + it('has aria-label when one is specified', () => { + render(content); + expect(screen.getByLabelText('Body label')).toBeVisible(); + }); + + it('has aria-labelledby when one is specified', () => { + const { container } = render(content); + expect(container.firstElementChild).toHaveAttribute('aria-labelledby', 'some-id'); + }); + + it('wrapper element is of type div when wrapperElement is not specified', () => { + const { container } = render(content); + expect(container.firstElementChild?.tagName).toEqual('DIV'); + }); + + it('renders with custom wrapperElement', () => { + const { container } = render(content); + expect(container.firstElementChild?.tagName).toEqual('MAIN'); + }); +}); diff --git a/packages/react-core/src/next/components/Wizard/__tests__/WizardFooter.test.tsx b/packages/react-core/src/next/components/Wizard/__tests__/WizardFooter.test.tsx new file mode 100644 index 00000000000..61b6e63a3da --- /dev/null +++ b/packages/react-core/src/next/components/Wizard/__tests__/WizardFooter.test.tsx @@ -0,0 +1,95 @@ +import React from 'react'; + +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import { WizardFooter } from '../WizardFooter'; + +describe('WizardFooter', () => { + const defaultProps = { + activeStep: { name: 'Step name', id: 'some-id' }, + onNext: jest.fn(), + onBack: jest.fn(), + onClose: jest.fn() + }; + + it('has button names of "Next", "Back", and "Cancel" by default', () => { + render(); + + expect(screen.getByRole('button', { name: 'Next' })).toBeVisible(); + expect(screen.getByRole('button', { name: 'Back' })).toBeVisible(); + expect(screen.getByRole('button', { name: 'Cancel' })).toBeVisible(); + }); + + it('calls onNext when the next button is clicked', async () => { + const onNext = jest.fn(); + const user = userEvent.setup(); + + render(); + + await user.click(screen.getByRole('button', { name: 'Next' })); + expect(onNext).toHaveBeenCalled(); + }); + + it('calls onBack when the back button is clicked', async () => { + const onBack = jest.fn(); + const user = userEvent.setup(); + + render(); + + await user.click(screen.getByRole('button', { name: 'Back' })); + expect(onBack).toHaveBeenCalled(); + }); + + it('calls onClose when the close button is clicked', async () => { + const onClose = jest.fn(); + const user = userEvent.setup(); + + render(); + + await user.click(screen.getByRole('button', { name: 'Cancel' })); + expect(onClose).toHaveBeenCalled(); + }); + + it('can have custom button names', () => { + render( + Go!} + backButtonText="Turn around!" + cancelButtonText={Get out!} + /> + ); + + expect(screen.getByRole('button', { name: 'Go!' })).toBeVisible(); + expect(screen.getByRole('button', { name: 'Turn around!' })).toBeVisible(); + expect(screen.getByRole('button', { name: 'Get out!' })).toBeVisible(); + }); + + it('has disabled back button when disableBackButton is enabled', () => { + render(); + expect(screen.getByRole('button', { name: 'Back' })).toHaveAttribute('disabled'); + }); + + it('has no back button when activeStep has hideBackButton enabled', () => { + render(); + expect(screen.queryByRole('button', { name: 'Back' })).toBeNull(); + }); + + it('has no cancel button when activeStep has hideCancelButton enabled', () => { + render(); + expect(screen.queryByRole('button', { name: 'Cancel' })).toBeNull(); + }); + + it(`uses activeStep's nextButtonText when specified instead of nextButtonText of WizardFooter`, () => { + render( + + ); + expect(screen.queryByRole('button', { name: 'Footer next' })).toBeNull(); + expect(screen.getByRole('button', { name: 'Active step next' })).toBeVisible(); + }); +}); diff --git a/packages/react-core/src/next/components/Wizard/__tests__/WizardStep.test.tsx b/packages/react-core/src/next/components/Wizard/__tests__/WizardStep.test.tsx new file mode 100644 index 00000000000..91719fe560c --- /dev/null +++ b/packages/react-core/src/next/components/Wizard/__tests__/WizardStep.test.tsx @@ -0,0 +1,35 @@ +import React from 'react'; +import { render, screen } from '@testing-library/react'; +import { WizardStep } from '../WizardStep'; + +describe('WizardStep', () => { + it('renders without children', () => { + const { container } = render(); + expect(container).toBeVisible(); + }); + + it('renders with children', () => { + render( + + content + + ); + + expect(screen.getByText('content')).toBeVisible(); + }); + + it('excludes WizardBody when body is set to null', () => { + render(); + expect(screen.queryByRole('main')).toBeNull(); + }); + + it('uses body props for WizardBody when passed', () => { + render( + + content + + ); + + expect(screen.getByLabelText('Some label')).toBeVisible(); + }); +}); diff --git a/packages/react-core/src/next/components/Wizard/__tests__/WizardToggle.test.tsx b/packages/react-core/src/next/components/Wizard/__tests__/WizardToggle.test.tsx new file mode 100644 index 00000000000..3af197e1192 --- /dev/null +++ b/packages/react-core/src/next/components/Wizard/__tests__/WizardToggle.test.tsx @@ -0,0 +1,80 @@ +import React from 'react'; + +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import { WizardToggle } from '../WizardToggle'; + +describe('WizardToggle', () => { + const steps = [ + { id: 'id-1', name: 'First step', component: <>First step content }, + { id: 'id-2', name: 'Second step', component: <>Second step content } + ]; + const defaultProps = { + steps, + activeStep: steps[0], + footer: <>Some footer, + goToStepByIndex: jest.fn() + }; + + it('renders provided footer and active step content', () => { + render(); + + expect(screen.getByText('Some footer')).toBeVisible(); + expect(screen.getByText('First step content')).toBeVisible(); + }); + + it('calls goToStepByIndex with step index when nav item is clicked', async () => { + const goToStepByIndex = jest.fn(); + const user = userEvent.setup(); + + render(); + + await user.click(screen.getByRole('button', { name: 'Second step' })); + expect(goToStepByIndex).toHaveBeenCalledWith(2); + }); + + it('renders with custom nav when nav is specified', () => { + render(
Some custom nav
} />); + expect(screen.getByText('Some custom nav')).toBeVisible(); + }); + + it('applies aria-label to the expandable toggle button when specified', () => { + render(); + expect(screen.getByRole('button', { name: 'Some label' })).toBeVisible(); + }); + + it('has expanded properties when the toggle dropdown button is clicked', async () => { + const user = userEvent.setup(); + + render(); + + const toggleButton = screen.getByRole('button', { name: 'Some label' }); + + expect(toggleButton).toHaveAttribute('aria-expanded', 'false'); + expect(toggleButton).not.toHaveClass('pf-m-expanded'); + + await user.click(toggleButton); + + expect(toggleButton).toHaveAttribute('aria-expanded', 'true'); + expect(toggleButton).toHaveClass('pf-m-expanded'); + }); + + it('unsets expanded properties when using the Escape key', async () => { + const user = userEvent.setup(); + + render(); + + const toggleButton = screen.getByRole('button', { name: 'Some label' }); + + await user.click(toggleButton); + + expect(toggleButton).toHaveAttribute('aria-expanded', 'true'); + expect(toggleButton).toHaveClass('pf-m-expanded'); + + await user.type(toggleButton, '{esc}'); + + expect(toggleButton).toHaveAttribute('aria-expanded', 'false'); + expect(toggleButton).not.toHaveClass('pf-m-expanded'); + }); +}); diff --git a/packages/react-core/src/next/components/Wizard/__tests__/utils.test.tsx b/packages/react-core/src/next/components/Wizard/__tests__/utils.test.tsx new file mode 100644 index 00000000000..3ac9fccc126 --- /dev/null +++ b/packages/react-core/src/next/components/Wizard/__tests__/utils.test.tsx @@ -0,0 +1,66 @@ +import React from 'react'; + +import { WizardParentStep, WizardSubStep } from '../types'; +import { buildSteps } from '../utils'; +import { WizardStep } from '../WizardStep'; + +describe('buildSteps', () => { + it('throws error if child does not have required props or type of WizardStep', () => { + try { + buildSteps(
); + } catch (error) { + expect(error.message).toEqual('Wizard only accepts children of type WizardStep'); + } + }); + + it('throws error if no children are valid react elements', () => { + try { + buildSteps('test' as any); + } catch (error) { + expect(error.message).toEqual('Wizard only accepts children of type WizardStep'); + } + }); + + it('returns array of steps if children are of type WizardStep', () => { + const component = ; + const [step] = buildSteps(component); + + expect(step.id).toEqual('step-1'); + expect(step.name).toEqual('Step 1'); + expect(step.component?.props).toEqual(component.props); + }); + + it('returns array of steps if children have required props', () => { + const CustomStep = props =>
; + const component = ( + + Content + + ); + const [step] = buildSteps(component); + + expect(step.id).toEqual('step-1'); + expect(step.name).toEqual('Step 1'); + expect(step.component?.props).toEqual(component.props); + }); + + it('returns flattened array of steps and sub-steps when sub-steps exist', () => { + const CustomSubStep = props =>
; + + const component = ( + , + + ]} + /> + ); + const [subStep, customSubStep, parentStep] = buildSteps(component); + + expect((subStep as WizardSubStep).parentId).toEqual('step-1'); + expect((customSubStep as WizardSubStep).parentId).toEqual('step-1'); + expect((parentStep as WizardParentStep).subStepIds).toEqual(['sub-step', 'custom-sub-step']); + }); +}); diff --git a/packages/react-core/src/next/components/Wizard/examples/Wizard.md b/packages/react-core/src/next/components/Wizard/examples/Wizard.md index 26af49822a8..565457c4a6a 100644 --- a/packages/react-core/src/next/components/Wizard/examples/Wizard.md +++ b/packages/react-core/src/next/components/Wizard/examples/Wizard.md @@ -47,8 +47,6 @@ WizardNavItem, WizardNav, WizardHeader } from '@patternfly/react-core/next'; -import { css } from '@patternfly/react-styles'; -import styles from '@patternfly/react-styles/css/components/Wizard/wizard'; PatternFly has two implementations of a `Wizard`. This newer `Wizard` takes a more explicit and declarative approach compared to the older implementation, which can be found under the [React](/components/wizard/react) tab. diff --git a/packages/react-core/src/next/components/Wizard/examples/WizardKitchenSink.tsx b/packages/react-core/src/next/components/Wizard/examples/WizardKitchenSink.tsx index cd78b3434b5..a54781be81f 100644 --- a/packages/react-core/src/next/components/Wizard/examples/WizardKitchenSink.tsx +++ b/packages/react-core/src/next/components/Wizard/examples/WizardKitchenSink.tsx @@ -27,8 +27,6 @@ import { WizardNavItem, WizardHeader } from '@patternfly/react-core/next'; -import { css } from '@patternfly/react-styles'; -import styles from '@patternfly/react-styles/css/components/Wizard/wizard'; const CustomWizardFooter = () => { const { activeStep, onNext, onBack, onClose } = useWizardContext(); @@ -100,14 +98,8 @@ const StepWithCustomFooter = () => { const footer = React.useMemo( () => ( -
- -
+ ), [isLoading, onBack, onClose, goToNextStep] ); diff --git a/packages/react-core/src/next/components/Wizard/hooks/__tests__/useWizardFooter.test.tsx b/packages/react-core/src/next/components/Wizard/hooks/__tests__/useWizardFooter.test.tsx new file mode 100644 index 00000000000..ba7121e68f6 --- /dev/null +++ b/packages/react-core/src/next/components/Wizard/hooks/__tests__/useWizardFooter.test.tsx @@ -0,0 +1,51 @@ +import React from 'react'; + +import { renderHook } from '@testing-library/react-hooks'; + +import { useWizardFooter } from '../useWizardFooter'; +import * as WizardContext from '../../WizardContext'; + +describe('useWizardFooter', () => { + const customFooter = <>My custom footer; + const useWizardContextSpy = jest.spyOn(WizardContext, 'useWizardContext'); + const setFooter = jest.fn(); + + it('sets the footer when one is provided without a stepId', () => { + useWizardContextSpy.mockReturnValueOnce({ setFooter } as any); + + renderHook(() => useWizardFooter(customFooter)); + expect(setFooter).toHaveBeenCalledWith(customFooter); + }); + + it(`sets the footer when the provided stepId matches the activeStep's id`, () => { + useWizardContextSpy.mockReturnValueOnce({ setFooter, activeStep: { id: 'active-step-id' } } as any); + + renderHook(() => useWizardFooter(customFooter, 'active-step-id')); + expect(setFooter).toHaveBeenCalledWith(customFooter); + }); + + it(`does not set the footer when the provided stepId does not match the activeStep's id`, () => { + useWizardContextSpy.mockReturnValueOnce({ setFooter, activeStep: { id: 'active-step-id' } } as any); + + renderHook(() => useWizardFooter(customFooter, 'some-other-step-id')); + expect(setFooter).not.toHaveBeenCalled(); + }); + + it('sets the footer to null on unmount', () => { + useWizardContextSpy.mockReturnValueOnce({ setFooter } as any); + + const { unmount } = renderHook(() => useWizardFooter(customFooter)); + unmount(); + + expect(setFooter).toHaveBeenCalledWith(null); + }); + + it('sets footer with default props instead of react element', () => { + const footerProps = { onNext: jest.fn(), onBack: jest.fn(), onClose: jest.fn() }; + useWizardContextSpy.mockReturnValueOnce({ setFooter } as any); + + renderHook(() => useWizardFooter(footerProps)); + + expect(setFooter).toHaveBeenCalledWith(footerProps); + }); +}); diff --git a/packages/react-core/src/next/components/Wizard/hooks/useWizardFooter.tsx b/packages/react-core/src/next/components/Wizard/hooks/useWizardFooter.tsx index 7910eec9c52..0d7c99b3515 100644 --- a/packages/react-core/src/next/components/Wizard/hooks/useWizardFooter.tsx +++ b/packages/react-core/src/next/components/Wizard/hooks/useWizardFooter.tsx @@ -1,4 +1,6 @@ import React from 'react'; + +import { DefaultWizardFooterProps } from '../types'; import { useWizardContext } from '../WizardContext'; /** @@ -6,7 +8,7 @@ import { useWizardContext } from '../WizardContext'; * @param footer * @param stepId */ -export const useWizardFooter = (footer: React.ReactElement, stepId?: string | number) => { +export const useWizardFooter = (footer: DefaultWizardFooterProps | React.ReactElement, stepId?: string | number) => { const { activeStep, setFooter } = useWizardContext(); React.useEffect(() => { diff --git a/packages/react-core/src/next/components/Wizard/types.tsx b/packages/react-core/src/next/components/Wizard/types.tsx index 768fdee4f5d..d386b78cf35 100644 --- a/packages/react-core/src/next/components/Wizard/types.tsx +++ b/packages/react-core/src/next/components/Wizard/types.tsx @@ -57,6 +57,12 @@ export interface DefaultWizardFooterProps { backButtonText?: React.ReactNode; /** The Cancel button text */ cancelButtonText?: React.ReactNode; + /** Next button callback */ + onNext?: () => WizardNavStepFunction | void; + /** Back button callback */ + onBack?: () => WizardNavStepFunction | void; + /** Cancel link callback */ + onClose?: () => void; } /** Encompasses all step type variants that are internally controlled by the Wizard. */ diff --git a/packages/react-core/src/next/components/Wizard/utils.ts b/packages/react-core/src/next/components/Wizard/utils.ts index 2ec93bb3890..071746f1319 100644 --- a/packages/react-core/src/next/components/Wizard/utils.ts +++ b/packages/react-core/src/next/components/Wizard/utils.ts @@ -4,7 +4,7 @@ import { WizardControlStep, WizardNavStepData } from './types'; import { WizardStep, WizardStepProps } from './WizardStep'; function hasWizardStepProps(props: WizardStepProps | any): props is WizardStepProps { - return props.name !== undefined && props.id !== undefined && props.children !== undefined; + return props.name !== undefined && props.id !== undefined; } /** @@ -14,29 +14,31 @@ function hasWizardStepProps(props: WizardStepProps | any): props is WizardStepPr */ export const buildSteps = (children: React.ReactElement | React.ReactElement[]) => React.Children.toArray(children).reduce((acc: WizardControlStep[], child) => { - if (React.isValidElement(child)) { - if (child.type === WizardStep || hasWizardStepProps(child.props)) { - // Omit "children" and use the whole "child" (WizardStep) for the component prop. Sub-steps will do the same. - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { steps: subSteps, id, children, ...stepProps } = child.props as WizardStepProps; + if (React.isValidElement(child) && (child.type === WizardStep || hasWizardStepProps(child.props))) { + // Omit "children" and use the whole "child" (WizardStep) for the component prop. Sub-steps will do the same. + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { steps: subSteps, id, children, ...stepProps } = child.props as WizardStepProps; - acc.push({ - id, - component: child, - ...stepProps, - ...(subSteps && { - subStepIds: subSteps?.map(subStep => { - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { children, ...subStepProps } = subStep.props; - acc.push({ ...subStepProps, component: subStep, parentId: id }); + acc.push({ + id, + component: child, + ...stepProps, + ...(subSteps && { + subStepIds: subSteps?.map(subStep => { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { children, ...subStepProps } = subStep.props; + acc.push({ + ...subStepProps, + component: subStep, + parentId: id + }); - return subStep.props.id; - }) + return subStep.props.id; }) - }); - } else { - throw new Error('Wizard only accepts children of type WizardStep'); - } + }) + }); + } else { + throw new Error('Wizard only accepts children of type WizardStep'); } return acc; diff --git a/yarn.lock b/yarn.lock index 8ce7dd3f5dd..fcd117df757 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3705,6 +3705,14 @@ lodash "^4.17.15" redent "^3.0.0" +"@testing-library/react-hooks@^8.0.1": + version "8.0.1" + resolved "https://registry.yarnpkg.com/@testing-library/react-hooks/-/react-hooks-8.0.1.tgz#0924bbd5b55e0c0c0502d1754657ada66947ca12" + integrity sha512-Aqhl2IVmLt8IovEVarNDFuJDVWVvhnr9/GCU6UUnrYXwgDFF9h2L2o2P9KBni1AST5sT6riAyoukFLyjQUgD/g== + dependencies: + "@babel/runtime" "^7.12.5" + react-error-boundary "^3.1.0" + "@testing-library/react@^12.1.5": version "12.1.5" resolved "https://registry.npmjs.org/@testing-library/react/-/react-12.1.5.tgz" @@ -14357,6 +14365,13 @@ react-dropzone@9.0.0: prop-types "^15.6.2" prop-types-extra "^1.1.0" +react-error-boundary@^3.1.0: + version "3.1.4" + resolved "https://registry.yarnpkg.com/react-error-boundary/-/react-error-boundary-3.1.4.tgz#255db92b23197108757a888b01e5b729919abde0" + integrity sha512-uM9uPzZJTF6wRQORmSrvOIgt4lJ9MC1sNgEOj2XGsDTRE4kmpWxg7ENK9EWNKJRMAOY9z0MuF4yIfl6gp4sotA== + dependencies: + "@babel/runtime" "^7.12.5" + react-fast-compare@^3.2.0: version "3.2.0" resolved "https://registry.yarnpkg.com/react-fast-compare/-/react-fast-compare-3.2.0.tgz#641a9da81b6a6320f270e89724fb45a0b39e43bb" From b36baebb429ecfcea3247c6a290bd4bc9412c981 Mon Sep 17 00:00:00 2001 From: Jeff Puzzo Date: Wed, 14 Sep 2022 12:17:50 -0400 Subject: [PATCH 2/2] refactor WizardToggle a bit more and address feedback --- .../src/next/components/Wizard/Wizard.tsx | 63 +- .../next/components/Wizard/WizardContext.tsx | 6 +- .../src/next/components/Wizard/WizardNav.tsx | 6 +- .../next/components/Wizard/WizardToggle.tsx | 121 ++-- .../Wizard/__tests__/Wizard.test.tsx | 654 +++++++++--------- .../Wizard/__tests__/WizardBody.test.tsx | 74 +- .../Wizard/__tests__/WizardFooter.test.tsx | 172 +++-- .../Wizard/__tests__/WizardStep.test.tsx | 48 +- .../Wizard/__tests__/WizardToggle.test.tsx | 139 ++-- .../Wizard/__tests__/utils.test.tsx | 10 +- .../next/components/Wizard/examples/Wizard.md | 6 +- .../Wizard/examples/WizardCustomNav.tsx | 4 +- .../src/next/components/Wizard/types.tsx | 2 +- 13 files changed, 668 insertions(+), 637 deletions(-) diff --git a/packages/react-core/src/next/components/Wizard/Wizard.tsx b/packages/react-core/src/next/components/Wizard/Wizard.tsx index 5c8b1733c8c..ceae05460d3 100644 --- a/packages/react-core/src/next/components/Wizard/Wizard.tsx +++ b/packages/react-core/src/next/components/Wizard/Wizard.tsx @@ -11,9 +11,9 @@ import { CustomWizardNavFunction } from './types'; import { buildSteps, normalizeNavStep } from './utils'; -import { useWizardContext, WizardContextProvider } from './WizardContext'; +import { WizardContextProvider } from './WizardContext'; import { WizardStepProps } from './WizardStep'; -import { WizardToggle } from './WizardToggle'; +import { WizardToggleInternal } from './WizardToggle'; /** * Wrapper for all steps and hosts state, including navigation helpers, within context. @@ -27,7 +27,7 @@ export interface WizardProps extends React.HTMLProps { header?: React.ReactNode; /** Wizard footer */ footer?: DefaultWizardFooterProps | React.ReactElement; - /** Default wizard nav props or a custom WizardNav (with callback) */ + /** Wizard nav */ nav?: DefaultWizardNavProps | CustomWizardNavFunction; /** The initial index the wizard is to start on (1 or higher). Defaults to 1. */ startIndex?: number; @@ -51,8 +51,23 @@ export interface WizardProps extends React.HTMLProps { onClose?: () => void; } -export const Wizard = (props: WizardProps) => { - const { startIndex = 1, children, footer, onNavByIndex, onNext, onBack, onSave, onClose, ...internalProps } = props; +export const Wizard = ({ + startIndex = 1, + children, + footer, + height, + width, + className, + header, + nav, + unmountInactiveSteps, + onNavByIndex, + onNext, + onBack, + onSave, + onClose, + ...wrapperProps +}: WizardProps) => { const [currentStepIndex, setCurrentStepIndex] = React.useState(startIndex); const steps = buildSteps(children); @@ -138,35 +153,19 @@ export const Wizard = (props: WizardProps) => { goToStepByName={goToStepByName} goToStepByIndex={goToStepByIndex} > - {children} +
+ {header} + +
); }; -// eslint-disable-next-line patternfly-react/no-anonymous-functions -const WizardInternal = ({ height, width, className, header, nav, unmountInactiveSteps, ...divProps }: WizardProps) => { - const { activeStep, steps, footer, goToStepByIndex } = useWizardContext(); - - return ( -
- {header} - -
- ); -}; - Wizard.displayName = 'Wizard'; diff --git a/packages/react-core/src/next/components/Wizard/WizardContext.tsx b/packages/react-core/src/next/components/Wizard/WizardContext.tsx index 7e93bd8914d..d9a934203de 100644 --- a/packages/react-core/src/next/components/Wizard/WizardContext.tsx +++ b/packages/react-core/src/next/components/Wizard/WizardContext.tsx @@ -35,7 +35,7 @@ export const WizardContext = React.createContext({} as WizardContextProps); interface WizardContextRenderProps { steps: WizardControlStep[]; activeStep: WizardControlStep; - footer: DefaultWizardFooterProps | React.ReactElement; + footer: React.ReactElement; onNext(): void; onBack(): void; onClose(): void; @@ -118,7 +118,9 @@ export const WizardContextProvider: React.FunctionComponent - {typeof children === 'function' ? children({ activeStep, steps, footer, onNext, onBack, onClose }) : children} + {typeof children === 'function' + ? children({ activeStep, steps, footer: wizardFooter, onNext, onBack, onClose }) + : children} ); }; diff --git a/packages/react-core/src/next/components/Wizard/WizardNav.tsx b/packages/react-core/src/next/components/Wizard/WizardNav.tsx index 9cdfcb86ab2..76e8f6cd58b 100644 --- a/packages/react-core/src/next/components/Wizard/WizardNav.tsx +++ b/packages/react-core/src/next/components/Wizard/WizardNav.tsx @@ -10,7 +10,7 @@ export interface WizardNavProps { /** Sets the aria-labelledby attribute on the nav element */ 'aria-labelledby'?: string; /** Whether the nav is expanded */ - isOpen?: boolean; + isExpanded?: boolean; /** True to return the inner list without the wrapping nav element */ returnList?: boolean; } @@ -19,7 +19,7 @@ export const WizardNav: React.FunctionComponent = ({ children, 'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledBy, - isOpen = false, + isExpanded = false, returnList = false }: WizardNavProps) => { const innerList =
    {children}
; @@ -30,7 +30,7 @@ export const WizardNav: React.FunctionComponent = ({ return (