Skip to content

Uses same recovery code visuals on loa3#992

Merged
el-mapache merged 2 commits intomasterfrom
ab-normalize-pkey-screens
Jan 27, 2017
Merged

Uses same recovery code visuals on loa3#992
el-mapache merged 2 commits intomasterfrom
ab-normalize-pkey-screens

Conversation

@el-mapache
Copy link
Copy Markdown
Contributor

Why: To present a consistent interface to the user

Screenshots:

Before:
screen shot 2017-01-25 at 3 19 50 pm

After:
screen shot 2017-01-25 at 3 38 31 pm

@andrewhughey I left the header the same. If it should change to match the text on the loa1 sign up recovery code screen, let me know.

**Why**: To present a consistent interface to the user
@el-mapache el-mapache self-assigned this Jan 26, 2017
@andrewhughey
Copy link
Copy Markdown
Contributor

Let's change the header to match the other personal key page. We're adding a new page for success and handoff to an agency that covers completing the process. Thanks!

Copy link
Copy Markdown
Contributor

@hursey013 hursey013 left a comment

Choose a reason for hiding this comment

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

lgtm after andrew's update

end
end

context 'with javascript enabled', js: true do
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I thought our accordions didn't require JS, per @esgoodman. Is that something we're going to change later? By that, I mean that the user should be able to collapse and expand the accordion without JS.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Something like this perhaps, which only uses CSS + HTML? https://codepen.io/abergin/pen/ihlDf

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I can do some digging, but I think it may be difficult to make an accordion that is CSS only, but also a11y friendly to screen readers and keyboards. I believe the USWDS accordion also relies on JS. I tested @el-mapache's implementation out which shows the information in it's expanded state for non-JS users which seems like a good fallback option.

@@ -0,0 +1,44 @@
shared_examples_for 'recovery code page' do
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The name of this file does not need the .spec

expect(page).to have_content(t('users.recovery_code.help_text'))
end
end
it_behaves_like 'recovery code page', @user
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think the @user argument is necessary here because it's not being passed in to the shared examples. By setting @user in the before block, the shared examples have access to it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is, I forgot to remove it. Thanks for catching

Copy link
Copy Markdown
Contributor

@monfresh monfresh left a comment

Choose a reason for hiding this comment

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

LGTM % the spec comments

**Why**: To keep consistency with our other recovery code management
screen
@el-mapache el-mapache merged commit 0e6b9c3 into master Jan 27, 2017
@el-mapache el-mapache deleted the ab-normalize-pkey-screens branch January 27, 2017 16:55
amoose pushed a commit that referenced this pull request Mar 7, 2017
* Uses same recovery code visuals on loa3

**Why**: To present a consistent interface to the user

* Renames spec helper, fixes heading i18n refs

**Why**: To keep consistency with our other recovery code management
screen
amoose pushed a commit that referenced this pull request Mar 8, 2017
* Uses same recovery code visuals on loa3

**Why**: To present a consistent interface to the user

* Renames spec helper, fixes heading i18n refs

**Why**: To keep consistency with our other recovery code management
screen
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants