-
Notifications
You must be signed in to change notification settings - Fork 166
LG-11803 Add Cancel button to the How to Verify Page #10330
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.
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
Changes from all commits
2e67685
6e9c67c
42538e6
488d91c
25c4ee0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,3 +87,4 @@ | |
| }, | ||
| ], | ||
| ) %> | ||
| <%= render 'idv/doc_auth/cancel', step: 'how_to_verify' %> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| require 'rails_helper' | ||
|
|
||
| RSpec.describe 'idv/how_to_verify/show.html.erb' do | ||
| selection = Idv::HowToVerifyForm::IPP | ||
| let(:idv_how_to_verify_form) do | ||
| Idv::HowToVerifyForm.new(selection: selection) | ||
| end | ||
|
|
||
| before do | ||
| allow(view).to receive(:user_signing_up?).and_return(false) | ||
|
|
||
| assign :idv_how_to_verify_form, idv_how_to_verify_form | ||
| render | ||
| end | ||
|
|
||
| context 'renders the show template with' do | ||
| it 'a title and info text' do | ||
| expect(rendered).to have_content(t('doc_auth.headings.how_to_verify')) | ||
| expect(rendered).to have_content(t('doc_auth.info.how_to_verify')) | ||
| end | ||
|
|
||
| it 'two options for verifying your identity' do | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is similar enough to the above test that I don't think it's needed |
||
| expect(rendered).to have_content(t('doc_auth.headings.verify_online')) | ||
| expect(rendered).to have_content(t('doc_auth.headings.verify_at_post_office')) | ||
| end | ||
|
|
||
| it 'a continue button' do | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit - can we change this to something like
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similar thought for the other 2 tests that proceed this
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ignore this comment based on the screenshot you provided above |
||
| expect(rendered).to have_button(t('forms.buttons.continue')) | ||
| end | ||
|
|
||
| it 'a troubleshooting section' do | ||
| expect(rendered).to have_content( | ||
| t('doc_auth.info.how_to_verify_troubleshooting_options_header'), | ||
| ) | ||
| expect(rendered).to have_link(t('doc_auth.info.verify_online_link_text')) | ||
| expect(rendered).to have_link(t('doc_auth.info.verify_at_post_office_link_text')) | ||
| end | ||
|
|
||
| it 'a cancel link' do | ||
| expect(rendered).to have_link(t('links.cancel')) | ||
| end | ||
| end | ||
| end | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit - can we change this to something like
it 'has expected title and info text'so it reads more naturally?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@svalexander Here is how it currently reads when you run the specs
Existing: renders the show template with a title and info text.
Your suggestion: renders the show template with has expected title and info text.
I initially had expected in each but thought about it being implicitly implied because there was a test for it. I am willing to change it but wondering with this extra information if you'd still like to see that change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh lol yes you're right, ignore my comments about updating language