Skip to content

LG-12656 Update how to verify page#10289

Merged
svalexander merged 22 commits intomainfrom
shannon/lg-12656-how-to-verify-design-updates
Mar 29, 2024
Merged

LG-12656 Update how to verify page#10289
svalexander merged 22 commits intomainfrom
shannon/lg-12656-how-to-verify-design-updates

Conversation

@svalexander
Copy link
Contributor

@svalexander svalexander commented Mar 22, 2024

🎫 Ticket

Link to the relevant ticket:
LG-12656

🛠 Summary of changes

Content updates to the how to verify page as well as removal of the radio buttons to be in line with the new design.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Run this app
  • Run the oidc-sinatra app
  • Visit http://localhost:9292/
  • Navigate to the how_to_verify page
  • Verify that content is correct and radio buttons have been removed
  • Click on the remote option and verify you are routed to correct page
  • Navigate back
  • Click on ipp option and verify you are route to correct page
  • Do same for es
  • Do same for fr

👀 Screenshots

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

en: Screen Shot 2024-03-29 at 2 44 32 PM
es: Screen Shot 2024-03-29 at 2 45 03 PM
fr: Screen Shot 2024-03-29 at 2 45 23 PM
mobile: Screen Shot 2024-03-29 at 2 46 08 PM

@svalexander svalexander requested review from a team and WheresHJ and removed request for a team March 26, 2024 17:13
@svalexander svalexander changed the title update layout and remove radio buttons LG-12656 Update how to verify page Mar 26, 2024
@svalexander svalexander requested review from a team and KeithNava March 27, 2024 21:49
@allis-green allis-green self-requested a review March 27, 2024 22:34
@allis-green
Copy link

We found an inconsistency in the original Spanish gengo translation:

"Tomará fotografías de tu identificación para verificar su identidad completamente en línea. La mayoría de los usuarios finalizan este proceso de una sola vez. "

should read...

"Tomará fotografías de su identificación para verificar su identidad completamente en línea. La mayoría de los usuarios finalizan este proceso de una sola vez. "

<%= f.radio_button(
</div>
<% end %>
<%= simple_form_for(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised this isn't triggering a form labeling accessibility test that was added in #9508, since my understanding is that if we have more than one form on a page, they need to be labelled. Does that apply 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.

added aria-labels as well as a test for the expectation in the code you linked

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.

Looking good! Small comments

</div>
<% end %>
<%= simple_form_for(
@idv_how_to_verify_form,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to get these two forms to render with separate id's? The rendered HTML has them with the same id: "new_idv_how_to_verify_form"

Copy link
Contributor

Choose a reason for hiding this comment

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

This sounds like a basic HTML validation thing we'd ideally be checking in some baseline checks like our expect_page_to_have_no_accessibility_violations helper. Is this page currently validated with that? Or is that helper not catching duplicate IDs?

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like we're running that exact helper on line 31 here.

I guess that helper isn't catching duplicate IDs?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, yeah. Well, doesn't need to block here. We can update the IDs and follow-up separately to see about incorporating an HTML validation check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should we add a follow up ticket?

Copy link
Contributor

Choose a reason for hiding this comment

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

Follow-up to enhance accessibility tests: #10362

<p><%= t('doc_auth.info.verify_at_post_office_instruction') %></p>
<p><%= t('doc_auth.info.verify_at_post_office_description') %></p>
</div>
<%= f.submit t('forms.buttons.continue_ipp'), class: 'display-block margin-y-5', outline: true %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want the submit button inside of the label element? I don't think you want interactive elements inside of a label? MDN Link

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for linking this documentation. I'm moving it into a different div, now just trying to figure out how to get the button to align with the block of text above it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok i was able to move the button so it aligns with the block of text...but it's not a super neat solution

let(:in_person_proofing_opt_in_enabled) { true }

it 'displays expected content and requires a choice' do
it 'displays expected content and navigates to choice' do
Copy link
Contributor

Choose a reason for hiding this comment

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

Good addition!

@gina-yamada
Copy link
Contributor

gina-yamada commented Mar 28, 2024

Confirmed changes in yml files will not affect other areas of the application. I requested access to see Figma but don't have access yet to confirm layout. Design will ultimately need to sign off, so not blocking. List of AC in ticket is entirely met except for "Create a follow up ticket to update with DOS translations"- but that is not blocking for this PR. Reach out after above comments are addressed for a final approval-but looks close.

@JackRyan1989
Copy link
Contributor

@18F/identity-joy-designers I'm seeing some slightly weird styling on mobile. I'm not sure how the buttons are supposed to align on mobile, should they left align with the text, or should they be centered?

Screenshot:

Screenshot 2024-03-29 at 9 53 56 AM

@carmenrosalop carmenrosalop requested review from carmenrosalop and removed request for allis-green March 29, 2024 17:04
Copy link

@allis-green allis-green left a comment

Choose a reason for hiding this comment

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

Translations reviewed and approved

dispositivo móvil o una manera fácil de tomar fotografías de su
identificación.
verify_at_post_office_description: Esta opción es mejor si no tiene un teléfono
para tomar fotografías de su identificación.

Choose a reason for hiding this comment

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

Looks good!

fotografías de su identificación.
verify_online_instruction: Tomará fotografías de su identificación para
verificar su identidad completamente en línea. La mayoría de los
usuarios finalizan este proceso de una sola vez.

Choose a reason for hiding this comment

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

Looks good

upload_from_computer: Continuer sur cet ordinateur
upload_from_phone: Utilisez votre téléphone pour prendre des photos
verify_at_post_office: Confirmez votre identité un bureau de poste
verify_at_post_office: Vérifiez votre identité en ligne

Choose a reason for hiding this comment

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

Looks good

quelqu’un qui se fait passer pour vous. %{link_html}'
getting_started_learn_more: Pour plus d’informations sur la vérification de votre identité
how_to_verify: Vous avez la possibilité de confirmer votre identité en ligne ou
how_to_verify: Vous avez la possibilité de vérifier votre identité en ligne ou

Choose a reason for hiding this comment

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

reviewed and approved

Copy link

@carmenrosalop carmenrosalop left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you, Shannon!

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.

LGTM! Great work!

@svalexander svalexander merged commit 6f9dc9f into main Mar 29, 2024
@svalexander svalexander deleted the shannon/lg-12656-how-to-verify-design-updates branch March 29, 2024 19:53
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.

7 participants