From ae63285c0d2ab5230b8c079869711603a7d71482 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Wed, 17 Apr 2024 13:30:23 -0400 Subject: [PATCH 1/6] fix(Wizard): added prop to focus content on next/back --- .../src/components/Wizard/Wizard.tsx | 15 ++++++++++ .../src/components/Wizard/WizardBody.tsx | 18 ++++++------ .../src/components/Wizard/WizardContext.tsx | 16 +++++++++-- .../src/components/Wizard/WizardNavItem.tsx | 28 +++++-------------- .../src/components/Wizard/WizardToggle.tsx | 2 +- .../Wizard/__tests__/Wizard.test.tsx | 8 +++--- .../examples/WizardWithCustomFooter.tsx | 6 ++-- 7 files changed, 53 insertions(+), 40 deletions(-) diff --git a/packages/react-core/src/components/Wizard/Wizard.tsx b/packages/react-core/src/components/Wizard/Wizard.tsx index 78a90a0cf20..d9ec1186b26 100644 --- a/packages/react-core/src/components/Wizard/Wizard.tsx +++ b/packages/react-core/src/components/Wizard/Wizard.tsx @@ -55,6 +55,10 @@ export interface WizardProps extends React.HTMLProps { onSave?: (event: React.MouseEvent) => void | Promise; /** Callback function to close the wizard */ onClose?: (event: React.MouseEvent) => void; + /** @beta Flag indicating whether the wizard content should be focused after the onNext or onBack callbacks + * are called. + */ + shouldFocusContentOnNextOrBack?: boolean; } export const Wizard = ({ @@ -72,11 +76,13 @@ export const Wizard = ({ onStepChange, onSave, onClose, + shouldFocusContentOnNextOrBack = false, ...wrapperProps }: WizardProps) => { const [activeStepIndex, setActiveStepIndex] = React.useState(startIndex); const initialSteps = buildSteps(children); const firstStepRef = React.useRef(initialSteps[startIndex - 1]); + const wrapperRef = React.useRef(null); // When the startIndex maps to a parent step, focus on the first sub-step React.useEffect(() => { @@ -85,6 +91,11 @@ export const Wizard = ({ } }, [startIndex]); + const focusMainContentElement = () => + setTimeout(() => { + wrapperRef?.current?.focus && wrapperRef.current.focus(); + }, 0); + const goToNextStep = (event: React.MouseEvent, steps: WizardStepType[] = initialSteps) => { const newStep = steps.find((step) => step.index > activeStepIndex && isStepEnabled(steps, step)); @@ -94,6 +105,7 @@ export const Wizard = ({ setActiveStepIndex(newStep?.index); onStepChange?.(event, newStep, steps[activeStepIndex - 1], WizardStepChangeScope.Next); + shouldFocusContentOnNextOrBack && focusMainContentElement(); }; const goToPrevStep = (event: React.MouseEvent, steps: WizardStepType[] = initialSteps) => { @@ -103,6 +115,7 @@ export const Wizard = ({ setActiveStepIndex(newStep?.index); onStepChange?.(event, newStep, steps[activeStepIndex - 1], WizardStepChangeScope.Back); + shouldFocusContentOnNextOrBack && focusMainContentElement(); }; const goToStepByIndex = ( @@ -157,6 +170,8 @@ export const Wizard = ({ goToStepById={goToStepById} goToStepByName={goToStepByName} goToStepByIndex={goToStepByIndex} + shouldFocusContentOnNextOrBack={shouldFocusContentOnNextOrBack} + mainWrapperRef={wrapperRef} >
{ const [hasScrollbar, setHasScrollbar] = React.useState(false); const [previousWidth, setPreviousWidth] = React.useState(undefined); - const wrapperRef = React.useRef(null); const WrapperComponent = component; - const { activeStep } = React.useContext(WizardContext); + const { activeStep, shouldFocusContentOnNextOrBack, mainWrapperRef } = React.useContext(WizardContext); const defaultAriaLabel = ariaLabel || `${activeStep?.name} content`; React.useEffect(() => { const resize = () => { - if (wrapperRef?.current) { - const { offsetWidth, offsetHeight, scrollHeight } = wrapperRef.current; + if (mainWrapperRef?.current) { + const { offsetWidth, offsetHeight, scrollHeight } = mainWrapperRef.current; if (previousWidth !== offsetWidth) { setPreviousWidth(offsetWidth); @@ -56,12 +55,12 @@ export const WizardBody = ({ const handleResizeWithDelay = debounce(resize, 250); let observer = () => {}; - if (wrapperRef?.current) { - observer = getResizeObserver(wrapperRef.current, handleResizeWithDelay); - const { offsetHeight, scrollHeight } = wrapperRef.current; + if (mainWrapperRef?.current) { + observer = getResizeObserver(mainWrapperRef.current, handleResizeWithDelay); + const { offsetHeight, scrollHeight } = mainWrapperRef.current; setHasScrollbar(offsetHeight < scrollHeight); - setPreviousWidth((wrapperRef.current as HTMLElement).offsetWidth); + setPreviousWidth((mainWrapperRef.current as HTMLElement).offsetWidth); } return () => { @@ -71,7 +70,8 @@ export const WizardBody = ({ return ( WizardStepType; /** Set step by ID */ setStep: (step: Pick & Partial) => void; + /** @beta Flag indicating whether the wizard content should be focused after the onNext or onBack callbacks + * are called. + */ + shouldFocusContentOnNextOrBack: boolean; + /** @beta Ref for main wizard content element. */ + mainWrapperRef: React.RefObject; } export const WizardContext = React.createContext({} as WizardContextProps); @@ -47,6 +53,8 @@ export interface WizardContextProviderProps { steps: WizardStepType[], index: number ): void; + shouldFocusContentOnNextOrBack: boolean; + mainWrapperRef: React.RefObject; } export const WizardContextProvider: React.FunctionComponent = ({ @@ -59,7 +67,9 @@ export const WizardContextProvider: React.FunctionComponent { const [currentSteps, setCurrentSteps] = React.useState(initialSteps); const [currentFooter, setCurrentFooter] = React.useState(); @@ -139,7 +149,9 @@ export const WizardContextProvider: React.FunctionComponent goToStepByIndex(null, steps, index), [goToStepByIndex, steps] - ) + ), + shouldFocusContentOnNextOrBack, + mainWrapperRef }} > {children} diff --git a/packages/react-core/src/components/Wizard/WizardNavItem.tsx b/packages/react-core/src/components/Wizard/WizardNavItem.tsx index 065ab6235e2..483191d2531 100644 --- a/packages/react-core/src/components/Wizard/WizardNavItem.tsx +++ b/packages/react-core/src/components/Wizard/WizardNavItem.tsx @@ -44,6 +44,7 @@ export const WizardNavItem = ({ content = '', isCurrent = false, isDisabled = false, + // eslint-disable-next-line @typescript-eslint/no-unused-vars isVisited = false, stepIndex, onClick, @@ -68,23 +69,6 @@ export const WizardNavItem = ({ console.error('WizardNavItem: When using an anchor, please provide an href'); } - const ariaLabel = React.useMemo(() => { - if (status === WizardNavItemStatus.Error || (isVisited && !isCurrent)) { - let label = content.toString(); - - if (status === WizardNavItemStatus.Error) { - label += `, ${status}`; - } - - // No need to signify step is visited if current - if (isVisited && !isCurrent) { - label += ', visited'; - } - - return label; - } - }, [content, isCurrent, isVisited, status]); - return (
  • {isExpandable ? ( @@ -127,9 +110,12 @@ export const WizardNavItem = ({ {content} {/* TODO, patternfly/patternfly#5142 */} {status === WizardNavItemStatus.Error && ( - - - + <> + , {status} + + + + )} )} diff --git a/packages/react-core/src/components/Wizard/WizardToggle.tsx b/packages/react-core/src/components/Wizard/WizardToggle.tsx index cf815ffbb08..16e7ccf5fa9 100644 --- a/packages/react-core/src/components/Wizard/WizardToggle.tsx +++ b/packages/react-core/src/components/Wizard/WizardToggle.tsx @@ -103,7 +103,7 @@ export const WizardToggle = ({
    -
    +
    {nav} {bodyContent}
    diff --git a/packages/react-core/src/components/Wizard/__tests__/Wizard.test.tsx b/packages/react-core/src/components/Wizard/__tests__/Wizard.test.tsx index b7ccc2fe279..70d733b10aa 100644 --- a/packages/react-core/src/components/Wizard/__tests__/Wizard.test.tsx +++ b/packages/react-core/src/components/Wizard/__tests__/Wizard.test.tsx @@ -411,7 +411,7 @@ test('incrementally shows/hides steps based on the activeStep when isProgressive await user.click(nextButton); expect( screen.getByRole('button', { - name: 'Test step 1, visited' + name: 'Test step 1' }) ).toBeVisible(); expect( @@ -429,12 +429,12 @@ test('incrementally shows/hides steps based on the activeStep when isProgressive await user.click(nextButton); expect( screen.getByRole('button', { - name: 'Test step 1, visited' + name: 'Test step 1' }) ).toBeVisible(); expect( screen.getByRole('button', { - name: 'Test step 2, visited' + name: 'Test step 2' }) ).toBeVisible(); expect( @@ -447,7 +447,7 @@ test('incrementally shows/hides steps based on the activeStep when isProgressive await user.click(backButton); expect( screen.getByRole('button', { - name: 'Test step 1, visited' + name: 'Test step 1' }) ).toBeVisible(); expect( diff --git a/packages/react-core/src/components/Wizard/examples/WizardWithCustomFooter.tsx b/packages/react-core/src/components/Wizard/examples/WizardWithCustomFooter.tsx index b24996c0569..0c63da6558c 100644 --- a/packages/react-core/src/components/Wizard/examples/WizardWithCustomFooter.tsx +++ b/packages/react-core/src/components/Wizard/examples/WizardWithCustomFooter.tsx @@ -40,12 +40,12 @@ const CustomStepTwoFooter = () => { return ( - + From ae016cd33c096def2ec7732d12d1869c58f9b5d0 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Wed, 17 Apr 2024 13:37:05 -0400 Subject: [PATCH 2/6] Added new example --- .../src/components/Wizard/examples/Wizard.md | 9 +++++++++ .../Wizard/examples/WizardFocusOnNextBack.tsx | 16 ++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 packages/react-core/src/components/Wizard/examples/WizardFocusOnNextBack.tsx diff --git a/packages/react-core/src/components/Wizard/examples/Wizard.md b/packages/react-core/src/components/Wizard/examples/Wizard.md index d8cf4731573..617082575b3 100644 --- a/packages/react-core/src/components/Wizard/examples/Wizard.md +++ b/packages/react-core/src/components/Wizard/examples/Wizard.md @@ -59,6 +59,15 @@ import layout from '@patternfly/react-styles/css/layouts/Bullseye/bullseye'; ```ts file="./WizardBasic.tsx" ``` +### Focus content on next/back + +To focus the main content element of the `Wizard`, pass in the `shouldFocusContentOnNextOrBack` property. It is recommended that this is passed in so that users can navigate through a `WizardStep` content in order. + +If a `WizardStep` is passed a `body={null}` property, you must manually handle focus. + +```ts file="./WizardFocusOnNextBack.tsx" +``` + ### Basic with disabled steps ```ts file="./WizardBasicDisabledSteps.tsx" diff --git a/packages/react-core/src/components/Wizard/examples/WizardFocusOnNextBack.tsx b/packages/react-core/src/components/Wizard/examples/WizardFocusOnNextBack.tsx new file mode 100644 index 00000000000..1ca4b71c84f --- /dev/null +++ b/packages/react-core/src/components/Wizard/examples/WizardFocusOnNextBack.tsx @@ -0,0 +1,16 @@ +import React from 'react'; +import { Wizard, WizardStep } from '@patternfly/react-core'; + +export const WizardFocusOnNextBack: React.FunctionComponent = () => ( + + + Step 1 content + + + Step 2 content + + + Review step content + + +); From 386180ba6b478589b8f9bfc6c4def061efd5055a Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Wed, 17 Apr 2024 13:40:10 -0400 Subject: [PATCH 3/6] Removed beta flag on context props --- packages/react-core/src/components/Wizard/WizardContext.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-core/src/components/Wizard/WizardContext.tsx b/packages/react-core/src/components/Wizard/WizardContext.tsx index c3eb29ee196..8fed97e25f7 100644 --- a/packages/react-core/src/components/Wizard/WizardContext.tsx +++ b/packages/react-core/src/components/Wizard/WizardContext.tsx @@ -28,11 +28,11 @@ export interface WizardContextProps { getStep: (stepId: number | string) => WizardStepType; /** Set step by ID */ setStep: (step: Pick & Partial) => void; - /** @beta Flag indicating whether the wizard content should be focused after the onNext or onBack callbacks + /** Flag indicating whether the wizard content should be focused after the onNext or onBack callbacks * are called. */ shouldFocusContentOnNextOrBack: boolean; - /** @beta Ref for main wizard content element. */ + /** Ref for main wizard content element. */ mainWrapperRef: React.RefObject; } From 5b5a690e2ea9c3d1945f6304639744098942bd26 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Fri, 19 Apr 2024 15:07:59 -0400 Subject: [PATCH 4/6] Removed aria-live attr --- packages/react-core/src/components/Wizard/WizardToggle.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-core/src/components/Wizard/WizardToggle.tsx b/packages/react-core/src/components/Wizard/WizardToggle.tsx index 16e7ccf5fa9..cf815ffbb08 100644 --- a/packages/react-core/src/components/Wizard/WizardToggle.tsx +++ b/packages/react-core/src/components/Wizard/WizardToggle.tsx @@ -103,7 +103,7 @@ export const WizardToggle = ({
    -
    +
    {nav} {bodyContent}
    From bf7318d31e5244de689d265f6cb7b12cea13e145 Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Fri, 26 Apr 2024 15:43:21 -0400 Subject: [PATCH 5/6] Updated prop name --- packages/react-core/src/components/Wizard/Wizard.tsx | 10 +++++----- .../react-core/src/components/Wizard/WizardBody.tsx | 4 ++-- .../react-core/src/components/Wizard/WizardContext.tsx | 8 ++++---- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/react-core/src/components/Wizard/Wizard.tsx b/packages/react-core/src/components/Wizard/Wizard.tsx index d9ec1186b26..6228c9a1507 100644 --- a/packages/react-core/src/components/Wizard/Wizard.tsx +++ b/packages/react-core/src/components/Wizard/Wizard.tsx @@ -58,7 +58,7 @@ export interface WizardProps extends React.HTMLProps { /** @beta Flag indicating whether the wizard content should be focused after the onNext or onBack callbacks * are called. */ - shouldFocusContentOnNextOrBack?: boolean; + shouldFocusContent?: boolean; } export const Wizard = ({ @@ -76,7 +76,7 @@ export const Wizard = ({ onStepChange, onSave, onClose, - shouldFocusContentOnNextOrBack = false, + shouldFocusContent = false, ...wrapperProps }: WizardProps) => { const [activeStepIndex, setActiveStepIndex] = React.useState(startIndex); @@ -105,7 +105,7 @@ export const Wizard = ({ setActiveStepIndex(newStep?.index); onStepChange?.(event, newStep, steps[activeStepIndex - 1], WizardStepChangeScope.Next); - shouldFocusContentOnNextOrBack && focusMainContentElement(); + shouldFocusContent && focusMainContentElement(); }; const goToPrevStep = (event: React.MouseEvent, steps: WizardStepType[] = initialSteps) => { @@ -115,7 +115,7 @@ export const Wizard = ({ setActiveStepIndex(newStep?.index); onStepChange?.(event, newStep, steps[activeStepIndex - 1], WizardStepChangeScope.Back); - shouldFocusContentOnNextOrBack && focusMainContentElement(); + shouldFocusContent && focusMainContentElement(); }; const goToStepByIndex = ( @@ -170,7 +170,7 @@ export const Wizard = ({ goToStepById={goToStepById} goToStepByName={goToStepByName} goToStepByIndex={goToStepByIndex} - shouldFocusContentOnNextOrBack={shouldFocusContentOnNextOrBack} + shouldFocusContent={shouldFocusContent} mainWrapperRef={wrapperRef} >
    (undefined); const WrapperComponent = component; - const { activeStep, shouldFocusContentOnNextOrBack, mainWrapperRef } = React.useContext(WizardContext); + const { activeStep, shouldFocusContent, mainWrapperRef } = React.useContext(WizardContext); const defaultAriaLabel = ariaLabel || `${activeStep?.name} content`; React.useEffect(() => { @@ -71,7 +71,7 @@ export const WizardBody = ({ return ( ; } @@ -53,7 +53,7 @@ export interface WizardContextProviderProps { steps: WizardStepType[], index: number ): void; - shouldFocusContentOnNextOrBack: boolean; + shouldFocusContent: boolean; mainWrapperRef: React.RefObject; } @@ -68,7 +68,7 @@ export const WizardContextProvider: React.FunctionComponent { const [currentSteps, setCurrentSteps] = React.useState(initialSteps); @@ -150,7 +150,7 @@ export const WizardContextProvider: React.FunctionComponent goToStepByIndex(null, steps, index), [goToStepByIndex, steps] ), - shouldFocusContentOnNextOrBack, + shouldFocusContent, mainWrapperRef }} > From a2061adbe0758f5cbccbef551d79d8e75a4e3b1a Mon Sep 17 00:00:00 2001 From: Eric Olkowski Date: Mon, 29 Apr 2024 09:09:28 -0400 Subject: [PATCH 6/6] Updated leftover instnaces of ...onNextOrBack prop name --- .../src/components/Wizard/examples/Wizard.md | 23 +++++++++++++++++-- .../Wizard/examples/WizardFocusOnNextBack.tsx | 2 +- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/react-core/src/components/Wizard/examples/Wizard.md b/packages/react-core/src/components/Wizard/examples/Wizard.md index 617082575b3..1dd41237c17 100644 --- a/packages/react-core/src/components/Wizard/examples/Wizard.md +++ b/packages/react-core/src/components/Wizard/examples/Wizard.md @@ -16,7 +16,7 @@ propComponents: 'WizardContextProps', 'WizardBasicStep', 'WizardParentStep', - 'WizardSubStep', + 'WizardSubStep' ] --- @@ -57,100 +57,119 @@ import layout from '@patternfly/react-styles/css/layouts/Bullseye/bullseye'; ### Basic ```ts file="./WizardBasic.tsx" + ``` ### Focus content on next/back -To focus the main content element of the `Wizard`, pass in the `shouldFocusContentOnNextOrBack` property. It is recommended that this is passed in so that users can navigate through a `WizardStep` content in order. +To focus the main content element of the `Wizard`, pass in the `shouldFocusContent` property. It is recommended that this is passed in so that users can navigate through a `WizardStep` content in order. If a `WizardStep` is passed a `body={null}` property, you must manually handle focus. ```ts file="./WizardFocusOnNextBack.tsx" + ``` ### Basic with disabled steps ```ts file="./WizardBasicDisabledSteps.tsx" + ``` ### Anchors for nav items ```ts file="./WizardWithNavAnchors.tsx" + ``` ### Incrementally enabled steps ```ts file="./WizardStepVisitRequired.tsx" + ``` ### Expandable steps ```ts file="./WizardExpandableSteps.tsx" + ``` ### Progress after submission ```ts file="./WizardWithSubmitProgress.tsx" + ``` ### Enabled on form validation ```ts file="./WizardEnabledOnFormValidation.tsx" + ``` ### Validate on button press ```ts file="./WizardValidateOnButtonPress.tsx" + ``` ### Progressive steps ```ts file="./WizardProgressiveSteps.tsx" + ``` ### Get current step ```ts file="./WizardGetCurrentStep.tsx" + ``` ### Within modal ```ts file="./WizardWithinModal.tsx" + ``` ### Step drawer content ```ts file="./WizardStepDrawerContent.tsx" + ``` ### Custom navigation ```ts file="./WizardWithCustomNav.tsx" + ``` ### Header ```ts file="./WizardWithHeader.tsx" + ``` ### Custom footer ```ts file="./WizardWithCustomFooter.tsx" + ``` ### Custom navigation item ```ts file="./WizardWithCustomNavItem.tsx" + ``` ### Toggle step visibility ```ts file="./WizardToggleStepVisibility.tsx" + ``` ### Step error status ```ts file="./WizardStepErrorStatus.tsx" + ``` ## Hooks diff --git a/packages/react-core/src/components/Wizard/examples/WizardFocusOnNextBack.tsx b/packages/react-core/src/components/Wizard/examples/WizardFocusOnNextBack.tsx index 1ca4b71c84f..8f037a1df37 100644 --- a/packages/react-core/src/components/Wizard/examples/WizardFocusOnNextBack.tsx +++ b/packages/react-core/src/components/Wizard/examples/WizardFocusOnNextBack.tsx @@ -2,7 +2,7 @@ import React from 'react'; import { Wizard, WizardStep } from '@patternfly/react-core'; export const WizardFocusOnNextBack: React.FunctionComponent = () => ( - + Step 1 content