feat(PF4): Wizard

[jschuler]

Adding the Wizard to PF4

[codecov-io]

Codecov Report

Merging #1539 into master will decrease coverage by 0.83%.
The diff coverage is 60.73%.

@@            Coverage Diff             @@
##           master    #1539      +/-   ##
==========================================
- Coverage    83.9%   83.06%   -0.84%     
==========================================
  Files         558      564       +6     
  Lines        5882     6101     +219     
  Branches       12       59      +47     
==========================================
+ Hits         4935     5068     +133     
- Misses        945     1010      +65     
- Partials        2       23      +21

Flag	Coverage Δ
#patternfly3	84.89% <ø> (ø)	⬆️
#patternfly4	79.65% <60.73%> (-1.99%)	⬇️
#patternflymisc	95.65% <ø> (ø)	⬆️

Impacted Files	Coverage Δ
.../react-core/src/components/Wizard/WizardToggle.tsx	53.84% <53.84%> (ø)
...nfly-4/react-core/src/components/Wizard/Wizard.tsx	55.19% <55.19%> (ø)
...react-core/src/components/Wizard/WizardNavItem.tsx	81.81% <81.81%> (ø)
...-4/react-core/src/components/Wizard/WizardBody.tsx	83.33% <83.33%> (ø)
...y-4/react-core/src/components/Wizard/WizardNav.tsx	90% <90%> (ø)
.../react-core/src/components/Wizard/WizardHeader.tsx	91.66% <91.66%> (ø)
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ba0357...c868384. Read the comment docs.

[patternfly-build]

PatternFly-React preview: https://1539-pr-patternfly-react-patternfly.surge.sh

[christiemolloy]

Some immediate things I noticed:

The colors in the nav don't match the colors in Core:

PR:

Core:

When the nav is expanded from the toggle on mobile, the toggle arrow icon should rotate 180 degrees:

PR:

Core:

[redallen]

Resolves #1548

[jschuler]

@christiemolloy Thanks, to the first one, that is because the nav items are not yet active and cannot be directly navigated to. Once you've hit the Next button a few times, those items are active and you can click on previous nav items to jump back and forth.
The second one I will fix :)

[jschuler]

@jgiardino Can you take a look at the a11y of this? Thanks!

[jschuler]

@mcoker Can you review the UX? THanks!

[mcoker]

There are 2 .pf-c-wizard__outer-wrap and .pf-c-wizard__inner-wrap elements when you expand the steps menu on mobile - there should only be 1 of each. I would just make sure the markup matches the example in core.

That should also fix this issue where when you expand the steps on mobile, the content is currently being pushed down. It shouldn't push down - the steps should display on top of the content.

[jschuler]

Thanks @mcoker this should be fixed now

[mcoker]

DOM structure looks great, thanks @jschuler. We have a .pf-m-no-padding modifier that applies to .pf-c-wizard__main that will remove padding if you want to put your own full-width layout in the main section. Is that supported as a prop? I don't see it.

[jschuler]

Cool thanks for making me aware of that, I'll add a prop to enable this

[jgiardino]

Great work on this! There are a couple of accessibility issues I noticed that can be improved:

1. Have the modal behave like the Modal component
   • Focus is trapped for the keyboard-only user already, which is great! We also need to trap focus for screen reader users. The Modal component does this by applying aria-hidden to the main <div> that wraps the page contents (and making sure that the Modal div is not included in that div).
   • The Modal component also includes several attributes on the main wrapping div:
     <div role="dialog" aria-label="Modal Header" aria-describedby="pf-modal-0" aria-modal="true" class="pf-c-modal-box">
     These same attributes should be included on .pf-c-wizard.
     • role="dialog" and aria-modal="true" are important for some screen readers to behave as expected
     • In the Modal component, aria-label repeats the string that is defined as the modal heading. We can also use aria-labelledby to reference the id of the heading. Either method is fine, but it should result in the string defined for this element being used as the label: .pf-c-wizard__title
     • The description element .pf-c-wizard__description should include an id so that aria-describedby on .pf-c-wizard can reference in
2. The navigation links currently are not accessible for either keyboard-only users or screen reader users. They can be made accessible by including a url in an href attribute.
3. In the mobile layout, the navigation is accessed as a menu, but there are a couple of issues related to that:
   • When the dropdown menu is visible, if I hit Esc then the modal dismisses when really I expect that the menu would dismiss and the modal would stay open
   • The dropdown behavior pattern that I think makes sense here currently isn't implemented anywhere yet. I included comments about similar behavior in the PRs for context selector and select components, but maybe it would make sense to have a quick chat with everyone to understand if there's a simpler way to handle this interaction so that it can be easily reused across the different components?

[ibolton336]

Looks great, @jschuler !! The product I am working on desperately needs this component in as we are racing against the clock to get something together for summit . @jgiardino Would it be possible to open a seperate issue to track the accessibility work related to this component? Thanks in advance

[jschuler]

I think I'll be able to address most of these today, the only one that might make more sense as a followup is the dropdown behavior pattern in 3).

[jgiardino]

@ibolton336 I totally understand. I captured my comments in this issue: #1588

/me refreshes the page and sees @jschuler's comment

That's awesome! I'll update the issue with what's left when this PR is done.

[jschuler]

Hi @jgiardino , here's what should be now addressed by my latest commit (once the build finishes should be able to see these):
The wizard container has these additional attributes:
role="dialog" aria-modal="true" aria-labelledby={this.titleId} aria-describedby={description ? this.descriptionId : null}

• titleId is the id set on the Wizard title
• descriptionId is the id set on the Wizard description

The wizard title also got an aria-label attribute set to the same Title

In mobile view, hitting the ESC key will first close the Nav dropdown if it's open

While the wizard is open, other elements in the document body should have the aria-hidden attribute set to true

[jgiardino]

Thanks, @jschuler! The updates noted in #1 in my comment are all addressed.

We could include a simple fix for #2 by adding href="#" to the links in the Steps navigation. This is considered bad practice by many, but that's also a huge can-o-worms discussion that we can have and revisit it later once we've settled on the right way to handle this.

[jschuler]

Do you mind if I address the href in the referenced issue? It is possible if not done right that when I add the href and the user hits the enter key, the URL at the top can change and the wizard could close. I think this needs a bit more time to make it fully work.