Skip to content
This repository has been archived by the owner on Feb 1, 2019. It is now read-only.

Wizard back navigation #347

Merged
merged 10 commits into from
Apr 15, 2015
Merged

Conversation

doelleri
Copy link
Contributor

For #320.

If you select a previously completed step, it is 'focused' and colored the same orange as when hovering over a step. The currently active step remains the blueish color when it is focused.

@@ -70,7 +70,7 @@
}
}

a { outline: 0; }
a { outline: 0; cursor: pointer; }
Copy link
Contributor

Choose a reason for hiding this comment

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

By default, anchors should already get cursor: pointer. What problem is this trying to solve?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An anchor without an href isn't getting a pointer. Which leads me to think I should probably change the element to a span.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in 415a855

@@ -2,8 +2,7 @@
<ol>
<li ng-repeat="step in steps" class="step {{step.stepClass}}">
<span class="{{step.badgeClass}}">{{step.badgeText}}</span>
<a class="step-title" ng-if="step.isComplete()" ng-href="/#/{{step.view}}">{{step.title}}</a>
<span class="step-title" ng-if="!step.isComplete()">{{step.title}}</span>
<span class="step-title" ng-click="setFocused(step)">{{step.title}}</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

While you can add an ng-click to a span, for accessibility we need to be presenting the user with an anchor. There are ways around that to continue using a span, but there's not reason why we can't optionally display the anchor here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about now? Switched back to an anchor in c0e7173.

poorgeek added a commit that referenced this pull request Apr 15, 2015
@poorgeek poorgeek merged commit c182691 into cfpb:master Apr 15, 2015
@doelleri doelleri mentioned this pull request Apr 15, 2015
@doelleri
Copy link
Contributor Author

This has been reverted manually. Was merged to master instead of milestone10.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants