Skip to content

chore(Wizard): updated API for WizardBody and docs for footer #10637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 17, 2024
Merged

chore(Wizard): updated API for WizardBody and docs for footer #10637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6367276
chore(Wizard): updated API for WizardBody and docs for footer
Jun 20, 2024
b2133b5
Updated examples to use ActionList in custom footers
Jun 20, 2024
3d92701
Updated integration tests for new default focus behavior
Jun 24, 2024
2b63af4
Removed conditional for rendering WizardBody
Jul 3, 2024
0039825
Yarnlock
Jul 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/react-core/src/components/Wizard/Wizard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export const Wizard = ({
onStepChange,
onSave,
onClose,
shouldFocusContent = false,
shouldFocusContent = true,
Copy link
Contributor Author

@thatblindgeye thatblindgeye Jun 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tlabaj I believe we had said that for v6 we wanted to make this the default, rather than an opt in, correct? If that's still the case then we could remove the example that shows this functionality, as well as update failing integration tests.

Copy link
Contributor

@tlabaj tlabaj Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is what was discussed for accessibility purposes

...wrapperProps
}: WizardProps) => {
const [activeStepIndex, setActiveStepIndex] = React.useState(startIndex);
Expand Down
4 changes: 2 additions & 2 deletions packages/react-core/src/components/Wizard/WizardStep.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ export interface WizardStepProps {
id: string | number;
/** Optional for when the step is used as a parent to sub-steps */
children?: React.ReactNode | undefined;
/** Props for WizardBody that wraps content by default. Can be set to null for exclusion of WizardBody. */
body?: Omit<Omit<WizardBodyProps, 'children'>, 'children'> | null;
/** Props for WizardBody that wraps content by default. */
body?: Omit<Omit<WizardBodyProps, 'children'>, 'children'>;
Copy link
Contributor

@wise-king-sullyman wise-king-sullyman Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the prop also be marked as required so that undefined can't be passed / the prop omitted? It sounds that way based on the issue, but I could be misunderstanding.

Copy link
Contributor Author

@thatblindgeye thatblindgeye Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it being optional should be fine. I'll try locally, but the logic in WizardToggle checks whether body || body === undefined to render the WizardBody wrapping children. We could probably just remove that conditional now,, though, since we should always want the WizardBody wrapper.

Copy link
Contributor Author

@thatblindgeye thatblindgeye Jul 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify, the body prop in WizardStep is for passing an object of WizardBody props in; the prop name isn't exactly descriptive of the actual behavior. May be worth updating the prop to bodyProps instead at some point.

/** Optional list of sub-steps */
steps?: React.ReactElement<WizardStepProps>[];
/** Flag to disable the step's navigation item */
Expand Down
3 changes: 1 addition & 2 deletions packages/react-core/src/components/Wizard/WizardToggle.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@ export const WizardToggle = ({

return (
<React.Fragment key={step.id}>
{activeStep?.id === step.id &&
(body || body === undefined ? <WizardBody {...body}>{children}</WizardBody> : children)}
{activeStep?.id === step.id && <WizardBody {...body}>{children}</WizardBody>}

<div key={step.id} style={{ display: 'none' }}>
<WizardStep {...propsWithoutChildren} />
Expand Down
5 changes: 0 additions & 5 deletions packages/react-core/src/components/Wizard/__tests__/WizardStep.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,6 @@ test('renders with children', () => {
expect(screen.getByText('content')).toBeVisible();
});

test('excludes WizardBody when body is set to null', () => {
render(<WizardStep id="test-step" name="Test step" body={null} />);
expect(screen.queryByRole('main')).toBeNull();
});

test('updates "isDisabled" in context when the value changes', () => {
render(<WizardStep {...testStep} isDisabled />);
expect(setStep).toHaveBeenCalledWith({ ...testStepProps, isDisabled: true, isVisited: true });
Expand Down
2 changes: 0 additions & 2 deletions packages/react-core/src/components/Wizard/examples/Wizard.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ import layout from '@patternfly/react-styles/css/layouts/Bullseye/bullseye';

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"

```
Expand Down
16 changes: 13 additions & 3 deletions packages/react-core/src/components/Wizard/examples/WizardStepDrawerContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,27 @@ const StepContentWithDrawer: React.FunctionComponent = () => {

export const WizardStepDrawerContent: React.FunctionComponent = () => (
<Wizard height={400} title="With drawer wizard">
<WizardStep body={null} name="Step 1" id="with-drawer-step-1">
<WizardStep body={{ hasNoPadding: true }} name="Step 1" id="with-drawer-step-1">
<StepContentWithDrawer />
</WizardStep>
<WizardStep
name="Step 2"
id="with-drawer-step-2"
steps={[
<WizardStep body={null} key="with-drawer-substep-1" name="Substep 1" id="with-drawer-substep-1">
<WizardStep
body={{ hasNoPadding: true }}
key="with-drawer-substep-1"
name="Substep 1"
id="with-drawer-substep-1"
>
<StepContentWithDrawer />
</WizardStep>,
<WizardStep body={null} key="with-drawer-substep-2" name="Substep 2" id="with-drawer-substep-2">
<WizardStep
body={{ hasNoPadding: true }}
key="with-drawer-substep-2"
name="Substep 2"
id="with-drawer-substep-2"
>
<StepContentWithDrawer />
</WizardStep>
]}
Expand Down
19 changes: 15 additions & 4 deletions packages/react-core/src/components/Wizard/examples/WizardValidateOnButtonPress.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ import React from 'react';

import {
Button,
ActionList,
ActionListGroup,
ActionListItem,
Alert,
EmptyState,
EmptyStateFooter,
Expand Down Expand Up @@ -96,10 +99,18 @@ const LastStepFooter: React.FunctionComponent<LastStepFooterProps> = ({

return (
<WizardFooterWrapper>
<Button onClick={onValidate}>Validate</Button>
<Button variant="secondary" onClick={goToPrevStep}>
Back
</Button>
<ActionList>
<ActionListGroup>
<ActionListItem>
<Button variant="secondary" onClick={goToPrevStep}>
Back
</Button>
</ActionListItem>
<ActionListItem>
<Button onClick={onValidate}>Validate</Button>
</ActionListItem>
</ActionListGroup>
</ActionList>
</WizardFooterWrapper>
);
};
Expand Down
33 changes: 24 additions & 9 deletions packages/react-core/src/components/Wizard/examples/WizardWithCustomFooter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import React from 'react';

import {
ActionList,
ActionListGroup,
ActionListItem,
Button,
Flex,
FlexItem,
Expand Down Expand Up @@ -40,15 +43,27 @@ const CustomStepTwoFooter = () => {

return (
<WizardFooterWrapper>
<Button variant="secondary" onClick={goToPrevStep} isDisabled={isLoading}>
Back
</Button>
<Button variant="primary" onClick={onNext} isLoading={isLoading} isDisabled={isLoading}>
Async Next
</Button>
<Button variant="link" onClick={close} isDisabled={isLoading}>
Cancel
</Button>
<ActionList>
<ActionListGroup>
<ActionListItem>
<Button variant="secondary" onClick={goToPrevStep} isDisabled={isLoading}>
Back
</Button>
</ActionListItem>
<ActionListItem>
<Button variant="primary" onClick={onNext} isLoading={isLoading} isDisabled={isLoading}>
Async Next
</Button>
</ActionListItem>
</ActionListGroup>
<ActionListGroup>
<ActionListItem>
<Button variant="link" onClick={close} isDisabled={isLoading}>
Cancel
</Button>
</ActionListItem>
</ActionListGroup>
</ActionList>
</WizardFooterWrapper>
);
};
Expand Down
15 changes: 10 additions & 5 deletions packages/react-core/src/demos/examples/Wizard/InModalWithDrawer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,25 +88,30 @@ export const WizardModalWithDrawerDemo: React.FunctionComponent = () => {
}
height={400}
>
<WizardStep body={null} name="Information" id="wizard-step-1">
<WizardStep body={{ hasNoPadding: true }} name="Information" id="wizard-step-1">
{createStepContentWithDrawer('Information step')}
</WizardStep>
<WizardStep
name="Configuration"
id="wizard-step-2"
steps={[
<WizardStep body={null} name="Substep A" id="wizard-step-2a" key="wizard-step-2a">
<WizardStep body={{ hasNoPadding: true }} name="Substep A" id="wizard-step-2a" key="wizard-step-2a">
{createStepContentWithDrawer('Configuration substep A')}
</WizardStep>,
<WizardStep body={null} name="Substep B" id="wizard-step-2b" key="wizard-step-2b">
<WizardStep body={{ hasNoPadding: true }} name="Substep B" id="wizard-step-2b" key="wizard-step-2b">
{createStepContentWithDrawer('Configuration substep B')}
</WizardStep>
]}
/>
<WizardStep body={null} name="Additional" id="wizard-step-3">
<WizardStep body={{ hasNoPadding: true }} name="Additional" id="wizard-step-3">
{createStepContentWithDrawer('Additional step')}
</WizardStep>
<WizardStep body={null} name="Review" id="wizard-step-4" footer={{ nextButtonText: 'Finish' }}>
<WizardStep
body={{ hasNoPadding: true }}
name="Review"
id="wizard-step-4"
footer={{ nextButtonText: 'Finish' }}
>
{createStepContentWithDrawer('Review step')}
</WizardStep>
</Wizard>
Expand Down
15 changes: 10 additions & 5 deletions packages/react-core/src/demos/examples/Wizard/InModalWithDrawerInformationalStep.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,25 +95,30 @@ export const WizardModalWithDrawerInfoStepDemo: React.FunctionComponent = () =>
}
height={400}
>
<WizardStep body={null} name="Information" id="wizard-step-1">
<WizardStep body={{ hasNoPadding: true }} name="Information" id="wizard-step-1">
{createStepContentWithDrawer('Information step')}
</WizardStep>
<WizardStep
name="Configuration"
id="wizard-step-2"
steps={[
<WizardStep body={null} name="Substep A" id="wizard-step-2a" key="wizard-step-2a">
<WizardStep body={{ hasNoPadding: true }} name="Substep A" id="wizard-step-2a" key="wizard-step-2a">
{createStepContentWithDrawer('Configuration substep A')}
</WizardStep>,
<WizardStep body={null} name="Substep B" id="wizard-step-2b" key="wizard-step-2b">
<WizardStep body={{ hasNoPadding: true }} name="Substep B" id="wizard-step-2b" key="wizard-step-2b">
{createStepContentWithDrawer('Configuration substep B')}
</WizardStep>
]}
/>
<WizardStep body={null} name="Additional" id="wizard-step-3">
<WizardStep body={{ hasNoPadding: true }} name="Additional" id="wizard-step-3">
{createStepContentWithDrawer('Additional step')}
</WizardStep>
<WizardStep body={null} name="Review" id="wizard-step-4" footer={{ nextButtonText: 'Finish' }}>
<WizardStep
body={{ hasNoPadding: true }}
name="Review"
id="wizard-step-4"
footer={{ nextButtonText: 'Finish' }}
>
{createStepContentWithDrawer('Review step')}
</WizardStep>
</Wizard>
Expand Down
15 changes: 10 additions & 5 deletions packages/react-core/src/demos/examples/Wizard/InPageWithDrawer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,25 +166,30 @@ export const WizardFullPageWithDrawerDemo: React.FunctionComponent = () => {
</PageSection>
<PageSection type={PageSectionTypes.wizard}>
<Wizard>
<WizardStep body={null} name="Information" id="wizard-step-1">
<WizardStep body={{ hasNoPadding: true }} name="Information" id="wizard-step-1">
{createStepContentWithDrawer('Information step')}
</WizardStep>
<WizardStep
name="Configuration"
id="wizard-step-2"
steps={[
<WizardStep body={null} name="Substep A" id="wizard-step-2a" key="wizard-step-2a">
<WizardStep body={{ hasNoPadding: true }} name="Substep A" id="wizard-step-2a" key="wizard-step-2a">
{createStepContentWithDrawer('Configuration substep A')}
</WizardStep>,
<WizardStep body={null} name="Substep B" id="wizard-step-2b" key="wizard-step-2b">
<WizardStep body={{ hasNoPadding: true }} name="Substep B" id="wizard-step-2b" key="wizard-step-2b">
{createStepContentWithDrawer('Configuration substep B')}
</WizardStep>
]}
/>
<WizardStep body={null} name="Additional" id="wizard-step-3">
<WizardStep body={{ hasNoPadding: true }} name="Additional" id="wizard-step-3">
{createStepContentWithDrawer('Additional step')}
</WizardStep>
<WizardStep body={null} name="Review" id="wizard-step-4" footer={{ nextButtonText: 'Finish' }}>
<WizardStep
body={{ hasNoPadding: true }}
name="Review"
id="wizard-step-4"
footer={{ nextButtonText: 'Finish' }}
>
{createStepContentWithDrawer('Review step')}
</WizardStep>
</Wizard>
Expand Down
15 changes: 10 additions & 5 deletions packages/react-core/src/demos/examples/Wizard/InPageWithDrawerInformationalStep.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,25 +171,30 @@ export const WizardFullPageWithDrawerInfoStepDemo: React.FunctionComponent = ()
</PageSection>
<PageSection type={PageSectionTypes.wizard} ß>
<Wizard>
<WizardStep body={null} name="Information" id="wizard-step-1">
<WizardStep body={{ hasNoPadding: true }} name="Information" id="wizard-step-1">
{createStepContentWithDrawer('Information step')}
</WizardStep>
<WizardStep
name="Configuration"
id="wizard-step-2"
steps={[
<WizardStep body={null} name="Substep A" id="wizard-step-2a" key="wizard-step-2a">
<WizardStep body={{ hasNoPadding: true }} name="Substep A" id="wizard-step-2a" key="wizard-step-2a">
{createStepContentWithDrawer('Configuration substep A')}
</WizardStep>,
<WizardStep body={null} name="Substep B" id="wizard-step-2b" key="wizard-step-2b">
<WizardStep body={{ hasNoPadding: true }} name="Substep B" id="wizard-step-2b" key="wizard-step-2b">
{createStepContentWithDrawer('Configuration substep B')}
</WizardStep>
]}
/>
<WizardStep body={null} name="Additional" id="wizard-step-3">
<WizardStep body={{ hasNoPadding: true }} name="Additional" id="wizard-step-3">
{createStepContentWithDrawer('Additional step')}
</WizardStep>
<WizardStep body={null} name="Review" id="wizard-step-4" footer={{ nextButtonText: 'Finish' }}>
<WizardStep
body={{ hasNoPadding: true }}
name="Review"
id="wizard-step-4"
footer={{ nextButtonText: 'Finish' }}
>
{createStepContentWithDrawer('Review step')}
</WizardStep>
</Wizard>
Expand Down
29 changes: 9 additions & 20 deletions packages/react-integration/cypress/integration/wizard.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,31 +24,20 @@ describe('Wizard Demo Test', () => {
});
});

it('Verify in-page wizard step content is focusable and has role only if content overflows', () => {
cy.get('#inPageWizWithOverflow .pf-v6-c-wizard__main').should('not.have.attr', 'tabindex');
it('Verify in-page wizard step content has role only if content overflows', () => {
cy.get('#inPageWizWithOverflow .pf-v6-c-wizard__main').should('not.have.attr', 'role');
cy.get('#inPageWizWithOverflow .pf-v6-c-wizard__main').click();
cy.get('#inPageWizWithOverflow .pf-v6-c-wizard__main').should('not.have.focus');
cy.get('#inPageWizWithOverflow #inPage-overflow-step-2.pf-v6-c-wizard__nav-link').click();
cy.get('#inPageWizWithOverflow .pf-v6-c-wizard__main').should('have.attr', 'tabindex');
cy.get('#inPageWizWithOverflow .pf-v6-c-wizard__main').should('have.attr', 'role').and('eq', 'region');
cy.get('#inPageWizWithOverflow .pf-v6-c-wizard__main').click();
cy.get('#inPageWizWithOverflow .pf-v6-c-wizard__main').should('have.focus');
});

it('Verify modal wizard step content is focusable only if content overflows', () => {
cy.get('#launchWizOverflow').click();
cy.get('#inModalWizWithOverflow.pf-v6-c-wizard').should('exist');
cy.get('#inModalWizWithOverflow .pf-v6-c-wizard__main').should('not.have.attr', 'tabindex');
cy.get('#inModalWizWithOverflow .pf-v6-c-wizard__main').should('not.have.attr', 'role');
cy.get('#inModalWizWithOverflow .pf-v6-c-wizard__main').click();
cy.get('#inModalWizWithOverflow .pf-v6-c-wizard__main').should('not.have.focus');
cy.get('#inModalWizWithOverflow #modal-overflow-step-2.pf-v6-c-wizard__nav-link').click();
cy.get('#inModalWizWithOverflow main.pf-v6-c-wizard__main').should('exist');
cy.get('#inModalWizWithOverflow main.pf-v6-c-wizard__main').should('have.attr', 'tabindex');
cy.get('#inModalWizWithOverflow main.pf-v6-c-wizard__main').click();
cy.get('#inModalWizWithOverflow main.pf-v6-c-wizard__main').should('have.focus');
cy.get('#inModalWizWithOverflow .pf-v6-c-wizard__close > button').click();
it('Verify in-page wizard step content receives focus only on next or back click', () => {
cy.get('#inPageFocusTest .pf-v6-c-wizard__main').should('not.have.focus');
cy.get('#inPageFocusTest .pf-v6-c-action-list__group button.pf-m-primary').click();
cy.get('#inPageFocusTest .pf-v6-c-wizard__main').should('have.focus');
cy.get('#inPageFocusTest #inPageFocusTest-wizard-step-b2').click();
cy.get('#inPageFocusTest .pf-v6-c-wizard__main').should('not.have.focus');
cy.get('#inPageFocusTest .pf-v6-c-action-list__group button.pf-m-secondary').click();
cy.get('#inPageFocusTest .pf-v6-c-wizard__main').should('have.focus');
});

it('Verify modal wizard roles are applied correctly', () => {
Expand Down
22 changes: 22 additions & 0 deletions packages/react-integration/demo-app-ts/src/components/demos/WizardDemo/WizardDemo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,28 @@ class WizardDemo extends React.Component<React.HTMLProps<HTMLDivElement>, Wizard
</Wizard>
<br />
<br />
<Wizard id="inPageFocusTest" height={500}>
<WizardStep name="A" id="inPageFocusTest-wizard-step-a">
<p>inPageFocusTest Step 1</p>
</WizardStep>
<WizardStep
name="B"
id="inPageFocusTest-wizard-step-b"
steps={[
<WizardStep name="B-1" id="inPageFocusTest-wizard-step-b1" key="inPageFocusTest-wizard-step-b1">
<p>inPageFocusTest Step 2</p>
</WizardStep>,
<WizardStep name="B-2" id="inPageFocusTest-wizard-step-b2" key="inPageFocusTest-wizard-step-b2">
<p>inPageFocusTest Step 3</p>
</WizardStep>
]}
/>
<WizardStep name="C" id="inPageFocusTest-wizard-step-c">
<p>inPageFocusTest Step 4</p>
</WizardStep>
</Wizard>
<br />
<br />
<Button id="launchWizOverflow" variant="primary" onClick={this.handleRoleWizardToggle}>
Show Modal with Overflow
</Button>
Expand Down
Loading