Skip to content

LG-9005 update verify info pg#8056

Merged
svalexander merged 16 commits intomainfrom
shannon/lg-9005-update-verify-info-pg
Mar 29, 2023
Merged

LG-9005 update verify info pg#8056
svalexander merged 16 commits intomainfrom
shannon/lg-9005-update-verify-info-pg

Conversation

@svalexander
Copy link
Contributor

@svalexander svalexander commented Mar 22, 2023

🎫 Ticket

Lg-9005

🛠 Summary of changes

This pr adds section titles for the verify your info page and adds the state id address information to the state-issued id section. These changes will only show for users for whom in_person_capture_secondary_id_enabled is true.
Shows address line 2 even if there is not value for it.

Does not currently impact team Ada. They are using app/views/idv/verify_info/show.html.erb for the verify view.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

To ensure updates are not seen:

  • Make sure in_person_capture_secondary_id_enabled is false
  • Walk through ipp flow and arrive at verify page, ensure no section titles are seen
    To check changes:
  • Update in_person_capture_secondary_id_enabled to true
  • Walk through ipp flow and arrive at verify page
  • Make sure section titles are visible and that address information is included in state id section
  • Make sure data input on state-id and address forms is correct

👀 Screenshots

If relevant, include a screenshot or screen capture of the changes.

Not enabled: 2023-03-22 at 6 07 12 PM
in_person_capture_secondary_id_enabled and same address: 2023-03-22 at 6 40 27 PM
in_person_capture_secondary_id_enabled and different address: 2023-03-28 at 11 36 16 AM

@svalexander svalexander marked this pull request as draft March 22, 2023 21:07
@svalexander svalexander marked this pull request as ready for review March 28, 2023 15:45
@svalexander svalexander requested review from a team, JackRyan1989 and WheresHJ and removed request for a team March 28, 2023 16:13
Comment on lines +75 to +96
<div>
<div>
<dt class="display-inline"> <%= t('idv.form.address1') %>: </dt>
<dd class="display-inline margin-left-0"> <%= pii[:address1] %> </dd>
</div>
<div>
<dt class="display-inline"> <%= t('idv.form.address2') %>: </dt>
<dd class="display-inline margin-left-0"> <%= pii[:address2] %> </dd>
</div>
<div>
<dt class="display-inline"> <%= t('idv.form.city') %>: </dt>
<dd class="display-inline margin-left-0"> <%= pii[:city] %> </dd>
</div>
<div>
<dt class="display-inline"> <%= t('idv.form.state') %>: </dt>
<dd class="display-inline margin-left-0"> <%= pii[:state] %> </dd>
</div>
<div>
<dt class="display-inline"> <%= t('idv.form.zipcode') %>: </dt>
<dd class="display-inline margin-left-0"> <%= pii[:zipcode] %> </dd>
</div>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same note.

Suggested change
<div>
<div>
<dt class="display-inline"> <%= t('idv.form.address1') %>: </dt>
<dd class="display-inline margin-left-0"> <%= pii[:address1] %> </dd>
</div>
<div>
<dt class="display-inline"> <%= t('idv.form.address2') %>: </dt>
<dd class="display-inline margin-left-0"> <%= pii[:address2] %> </dd>
</div>
<div>
<dt class="display-inline"> <%= t('idv.form.city') %>: </dt>
<dd class="display-inline margin-left-0"> <%= pii[:city] %> </dd>
</div>
<div>
<dt class="display-inline"> <%= t('idv.form.state') %>: </dt>
<dd class="display-inline margin-left-0"> <%= pii[:state] %> </dd>
</div>
<div>
<dt class="display-inline"> <%= t('idv.form.zipcode') %>: </dt>
<dd class="display-inline margin-left-0"> <%= pii[:zipcode] %> </dd>
</div>
</div>
<div>
<dt class="display-inline"> <%= t('idv.form.address1') %>: </dt>
<dd class="display-inline margin-left-0"> <%= pii[:address1] %> </dd>
</div>
<div>
<dt class="display-inline"> <%= t('idv.form.address2') %>: </dt>
<dd class="display-inline margin-left-0"> <%= pii[:address2] %> </dd>
</div>
<div>
<dt class="display-inline"> <%= t('idv.form.city') %>: </dt>
<dd class="display-inline margin-left-0"> <%= pii[:city] %> </dd>
</div>
<div>
<dt class="display-inline"> <%= t('idv.form.state') %>: </dt>
<dd class="display-inline margin-left-0"> <%= pii[:state] %> </dd>
</div>
<div>
<dt class="display-inline"> <%= t('idv.form.zipcode') %>: </dt>
<dd class="display-inline margin-left-0"> <%= pii[:zipcode] %> </dd>
</div>

@JackRyan1989
Copy link
Contributor

@svalexander - If I go through the flow selecting that my state id is separate from my residential address, and get to the verify your information page, and then select "Update" for my State Issued ID and change my response on the radio button from "No" to "yes"; meaning that my address is the same as on my ID, the information on the Verify My Information page is not updated. Meaning the distinct addresses are still listed.

Screen Shot 2023-03-28 at 3 04 18 PM

Is that outside of the scope of this ticket?

@svalexander
Copy link
Contributor Author

@svalexander - If I go through the flow selecting that my state id is separate from my residential address, and get to the verify your information page, and then select "Update" for my State Issued ID and change my response on the radio button from "No" to "yes"; meaning that my address is the same as on my ID, the information on the Verify My Information page is not updated. Meaning the distinct addresses are still listed.

Is that outside of the scope of this ticket?

@JackRyan1989 yeh I think we're handling that next sprint in LG-9141

Copy link
Contributor

@JackRyan1989 JackRyan1989 left a comment

Choose a reason for hiding this comment

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

Super small HTML change, but looks good!

<dt class="display-inline"> <%= t('idv.form.address2') %>: </dt>
<dd class="display-inline margin-left-0"> <%= pii[:address2] %> </dd>
</div>
<div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Super nit: do dt elements need to be enclosed by dl elements? MDN seems to say yes: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/dt

Copy link
Contributor Author

@svalexander svalexander Mar 28, 2023

Choose a reason for hiding this comment

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

the parent of the div this is in is a dl element so I think that follows the rec.

Copy link
Contributor

@JackRyan1989 JackRyan1989 left a comment

Choose a reason for hiding this comment

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

Tests pass locally, UI changes look ok locally. LGTM!

@WheresHJ
Copy link

One minor comment: We should increase the spacing between the section titles (State issued ID, Current residential address) and the first list item by 8px, to better match the Figma. I know its a really small thing, but we appreciate it! Otherwise, LGTM! Thanks!

@svalexander svalexander merged commit 2fe1b63 into main Mar 29, 2023
@svalexander svalexander deleted the shannon/lg-9005-update-verify-info-pg branch March 29, 2023 16:32
@aduth aduth mentioned this pull request Mar 29, 2023
@svalexander svalexander changed the title Shannon/lg 9005 update verify info pg LG-9005 update verify info pg Mar 29, 2023
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