Wizard ui match design

[mcoker]

fix #1044

(previously pr patternfly/patternfly#1045. created a new one to separate code change and LESS linting commits)

Description

better align wizard ui to design spec.

Changes

1. add completed step css
2. disable active hover state
3. fix mobile active step css
4. align grey and blue lines between steps in mobile
5. fix disappearing grey line between steps on mobile
6. add keyup event listener for required fields instead of relying on change

Link to rawgit and/or image

https://rawgit.com/michael-coker/patternfly/wizard-ui-match-design-dist/dist/tests/

PR checklist (if relevant)

• Cross browser: works in IE9
• Cross browser: works in IE10
• Cross browser: works in IE11
• Cross browser: works in Edge
• Cross browser: works in Chrome
• Cross browser: works in Firefox
• Cross browser: works in Safari
• Cross browser: works in Opera
• Responsive: works in extra small, small, medium and large view ports.
• Preview the PR: An image or a URL for designer to preview this PR is provided.

[mcoker]

@jeff-phillips-18 added the rawgit test URL

[matthewcarleton]

@michael-coker do you have access to browser stack to do all the testing?

[mcoker]

@matthewcarleton nope.

[matthewcarleton]

ok I've just invited you. There should be a checklist when you open a new PR for all the different browsers/devices that we support.

[mcoker]

@matthewcarleton great, thanks! Tested across all browsers and looks good except for the pre-existing flexbox issues in ie9, and there was a pre-existing issue with the main step circle/number in IE10, but I just pushed a fix for that.

Here are screenshots:

old & busted

new hotness

[mcoker]

ping for review @mcarrano @matthewcarleton @jgiardino

[mcarrano]

Looks good to me @michael-coker !

[jgiardino]

Looks good!

I just have one question about the expected behavior of the completed steps. Is the completed styling just meant to be applied to all steps that precede the current step? Or are they actually meant to indicate that a step was completed by the user (and what's the determining factor in marking it as completed)?

For example, if a step is completed, and I go back to a previous step, should the completed steps retain the blue border? If I have no required fields in the first step, skip it and move on to step 2, does step 1 display as completed?

In the current implementation, if I navigate back, the steps that I had completed return to their uncompleted styling.

I don't necessarily expect the core example to support any major logic flows, but when this component is added to a JS repo, these are some questions that will come up. And I just want to make sure the css supports the intended workflow.

[mcoker]

@jgiardino that's a great question. I'm inclined to say any step you've moved beyond is completed, and anything after the current step is uncompleted, even if you've put information in it. But this issue is the result of updating the wizard pattern design in PR patternfly/patternfly-design#601. Maybe @serenamarie125 @nlcwong or @mcarrano could chime in?

[mcarrano]

I think it gets complicated to define what "completed" means. I was assuming this just meant you had advanced beyond that step, but I'm interested to hear from others who have used the wizard within a product context. @cshinn I believe you were one of the people that worked on this design. Did you have an intention here?

[cshinn]

@mcarrano while I do think we have some work to do with respect to completed items (maybe some kind of emblem to show that there are required items unfilled on a given step, I'm inclined to say that for this initial version the blue border should recede to where you move. especially because something you do on an early may affect what comes on later steps.

[mcarrano]

That's fine @cshinn . But to clarify, I think what you are saying is that if I had advance to Step 4, but then reverted back to Step 2, the blue border would be removed from Step 3 since even though it was visited before, I will need to go back through that step again to account for changes I might have made since the first visit. Is that right?

@michael-coker let us know if you have enough guidance to finalize this.

[cshinn]

@mcarrano yeah, that's what I meant. I'd assume that everything you filled in should still be there if applicable, though.

[mcoker]

Thanks @cshinn @mcarrano. That interaction should be reflected in the UI from this PR - https://rawgit.com/michael-coker/patternfly/wizard-ui-match-design-dist/dist/tests/wizard.html

Is there anything with that demo that you'd like me to change or re-visit?

[mcarrano]

The only thing that I notice is that as I click through, looks like the Next and Back buttons are always showing as disabled even though they are clickable. Can you check this @michael-coker ?

[serenamarie125]

Should we be taking care of tab order here as well ? I recently created issues in Angular and React

[jgiardino]

Should we be taking care of tab order here as well ?

Good catch! I do think we should order elements in the DOM so that primary actions are focused first. With our most recent GMS workshop, one of our participants had commented totally unprompted that she would put Next before Previous (in the context of a pagination control) since she is most likely to choose Next.

[mcoker]

@mcarrano yep! You can also move past the first required field without entering anything by clicking the step # in the progress bar. I'll see if I can address both of those.

@serenamarie125 @jgiardino I'll address the tab order, too. Should I address the keyboard shortcuts as well?

[jgiardino]

Should I address the keyboard shortcuts as well?

I feel like this is a larger discussion that would be impossible to come to any decision on in a PR discussion thread. I'm trying to think of an upcoming meeting we might have where this would be a good topic to bring up, but the only meeting I think you and I would be involved in are the html/css discussions. Maybe we start there? Unless @mcarrano or @serenamarie125 want to kick it off in a design meeting? Also, by keyboard shortcuts, I assume you mean additional keyboard support enabled via JS, beyond what's supported by default with standard HTML.

[mcoker]

@jgiardino sounds good!

Also, by keyboard shortcuts, I assume you mean additional keyboard support enabled via JS, beyond what's supported by default with standard HTML.

Yeah, this part of the description in patternfly/angular-patternfly#738, though maybe I misread it?

Thus, if no input fields exist on a "page" in the wizard, Next should be the default button, thus hitting Enter would activate it.

In the wizard example, I'm assuming that would be the "Review" step where all you have is text (no <input> elements) and "Deploy" is the primary action/button, so hitting "enter" would behave like clicking the "Deploy" button.

[jgiardino]

Oh, thanks for the clarification, @michael-coker.

Maybe we can use standard html to associate the Next button as a submit button which would implicitly submit the form on Enter, which in our case is moving to the next page. Since we're placing focus in the first field when you move to a new page, maybe for the summary page we would automatically place focus on the Deploy button.

But if focus is not on the Deploy button, I'm not sure that we should catch the Enter keypress and trigger the Deploy button in that case. Something about that seems potentially dangerous and not obvious to the user. And except in cases when using the screen reader, I would assume that if focus isn't on the Deploy button, it's likely on one of the other buttons, which would make that unnecessary.

Something that we're doing for disabled buttons with patternfly-next is to include disabled="disabled". This implicitly pulls disabled buttons out of the tab order, which is a nice feature. And it also communicates to assistive technologies that the action is unavailable. Should we start adding this to PF3 examples?

[jeff-phillips-18]

Should we start adding this to PF3 examples?

IMO, Absolutely. I've notice several places where buttons look disabled but still get focus and still act like they are pressed when clicked

[mcoker]

Totally agreed re: using the disabled attribute. That will bake in the disabled/enabled functionality of the button and handle the design, too. Currently we're using the .disabled class, which only impacts the design, so we have to enable/disable the button functionality manually.

@jgiardino I agree with putting focus on the deploy button, instead of just capturing the enter key on that page.

[mcarrano]

@jgiardino @michael-coker The question about keyboard shortcuts is an interesting one that I think relates not only to accessibility but overall usability in general. You are right that we have not typically specified these as part of a design, but maybe we need to in the future. Let me bring this topic up in my weekly meeting with the UX Strategists. But Agree that this is a bigger discussion than what we can address with this PR.

[mcoker]

@jgiardino @mcarrano I addressed the tab order and enabled the wizard step to advance when you hit enter (capturing the submit event on the current step's <form>). Also brought the "deploy" button into focus on the review step. Do you want to check out the travis build and give me your feedback?

[jgiardino]

I'm sorry, I didn't fully process the UI suggestions from @serenamarie125 that @jeff-phillips-18 referenced in his previous comment. Just throwing my $.02 in about removing items from tab order...

In reading up on keyboard and screen reader accessibility, something that's often warned against is to have different items accessible to different types of users, because this can lead to confusion when instructions are given based on one user's perspective. My suggestion would be to keep the X and steps in tab order. If the user is going through the usual flow from start to finish, these actions aren't in their way, assuming that we place focus on the first field when a new form is loaded in the wizard.

@serenamarie125 and I had chatted about changing the tab order from how things are visually presented on the screen. Since we have control via css on where to position things, it's possible to structure the DOM so that primary actions precede secondary actions, even if visually they don't. So in this case, since Next is the primary action. we can have that be the first button that receives focus.

Related to this idea, when we had a recent activity with GMS students (read Stacy's blog post about it), one of the students said, unprompted, that she would separate the Previous and Next actions so that she can make Next first, which is most likely what she'd want to do.

[mcoker]

Since we have control via css on where to position things, it's possible to structure the DOM so that primary actions precede secondary actions, even if visually they don't. So in this case, since Next is the primary action. we can have that be the first button that receives focus.

This is how I addressed the tab order. The order in the HTML is next, back, cancel, and close. I'm using flexbox order to re-order them visually.

[jgiardino]

Going back to @mcarrano's previous comment:

@michael-coker Tab order looks good. The enter key does not advance the wizard as you describe, but actually closes it for me. I wonder how much we should get hung up here on programming specific button behaviors as some of these decisions might be application dependent. For example, once I get to the final screen, it probably wouldn't make sense to have a back button anymore since you've already performed the deployment. But these are really design decisions that I would expect someone working on product to make.

The enter key is working as I expect it to. When focus is on a form field, and I hit Enter, I see the next form. If focus isn't on a form field, then hitting Enter would trigger whatever is associated with the Enter key for the item that has focus.

Although I think I found a bug where something I did made the Next button go away, and I can't get it back. I did go all the way to the final page, then click on the step selectors to go back. And now when I reload the wizard, I still don't see next.

@jgiardino what's your take? How far do you think it's worth getting into workflow here? If you were consuming this for product would you consider that helpful or does it get in the way?

If I came across an example where the interaction was implemented, I would assume that the interaction matched what the designers intended.

[mcoker]

The enter key is working as I expect it to.

@mcarrano was loading a cached version of the page. He cleared his cache and verified that hitting enter submits the forms and advances the wizard as expected.

Although I think I found a bug where something I did made the Next button go away, and I can't get it back. I did go all the way to the final page, then click on the step selectors to go back.

Good catch. When you complete the wizard, should you be allowed to go back through the wizard and see your completed steps? After completing the wizard, the "back" button is disabled, and that's reflected in the wizard design, too, although it looks like disabling the back button is optional based on:

If the user has the option to go back and make alterations and resubmit the process, then the Back button should be enabled.

So it seems like we should choose whether the example allows folks to go back and re-submit, or not, and update the UI accordingly.

And now when I reload the wizard, I still don't see next.

Just to confirm, you mean you close the wizard, then launch it again without reloading the page? If so, I see that, too. It looks like the wizard will reload in the state it was when you closed it - text input values, buttons, etc. I'll update it to re-initialize everything when you hit the button to launch the wizard.

[mcoker]

Oh one more thing I noticed on the wizard design @jgiardino @serenamarie125. On the "review/summary" step, the "back" button is disabled. Is that by design, or should that button still be enabled? I would expect it to be enabled, so I can go back and make changes after looking at the review step.

[mcoker]

@jgiardino @mcarrano @serenamarie125 were there any other changes I should make to this PR?

[mcarrano]

@michael-coker right now it does seem like the buttons can get out of sync with the steps in the wizard. I haven't been able to reproduce this in a systematic way. It just seems a bit unpredictable. Does the flow seem to be correct for you? Regarding the correct behavior of the buttons, I agree that Back should always be available from the Review step, but may or may not be after the Deploy (or any Finish button) is clicked. I think in most cases it would not be.

[mcoker]

@mcarrano hmm. On Monday I addressed this issue from @jgiardino

Although I think I found a bug where something I did made the Next button go away, and I can't get it back. I did go all the way to the final page, then click on the step selectors to go back.

After that, the buttons and form fields behave as I would expect. Could you ensure you've cleared the cache and are loading the most recent version of the test page? And if you can reproduce it, that would be great.

Also made sure you can go back from the review step and disabled any backwards navigation after deploy.

[mcarrano]

I looked at this again @michael-coker . The problem seems to come when I get all the way to the end of the wizard and are looking at this screen:

If I then click the Back button I then get into a funky state as evidenced by this screen:

In my opinion, once you reach that last screen there should be nothing to do but navigate away or close. Clicking Back no longer makes sense since I've already committed to the operation, right?

[mcoker]

@mcarrano that's odd. Are you using FF? I've been testing primarily in Chrome, but tested across the browsers and everything looked ok. This is what I see in FF, and it looks quite a bit different than your screenshots.

[mcoker]

@jgiardino @jeff-phillips-18 @mcarrano could I get another review of this since addressing last bug @mcarrano found?

[jgiardino]

I'm no longer able to reproduce the issues I had noted before, and I see that options to go back are disabled.

[mcoker]

Thanks @jgiardino! @mcarrano is it ok to go ahead and merge this?

[mcoker]

@jeff-phillips-18 @mcarrano @jgiardino just pinging again to see if y'all are OK merging this? If so, would a couple of you mind approving it and I'll go ahead and merge unless one of you want to.

[mcarrano]

I am good from a design perspective, but since this is a code change, I think one of the developers should merge.

[mcoker]

@mcarrano thanks! @jeff-phillips-18 approved it last week, fyi. Jeff, do mind merging this when you get a chance? I haven't committed anything new since your review.