-
Notifications
You must be signed in to change notification settings - Fork 111
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
feat: updated translations for DOS IPP #10428
feat: updated translations for DOS IPP #10428
Conversation
cadcce9
to
543ea19
Compare
@KeithNava This should be the link to your review app |
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.
- Translations are updated as I'd expect on the page- see screenshots I added
- Checked that all of these references are not used outside of the how to verify page so these changes won't accidentally update content on other pages
- Approved but there needs to be several period deleted based on the other translations and where the text appears on the page
@KeithNava I do not feel comfortable with the change you pushed up to application.yml. It will turn on our feature flag if this gets merged in without being deleted. I am going to remove my approval until this get resolved. Please reach out when this is ready to reviewed again |
The translations content LGTM but I agree with Gina's feedback re: period consistency in the Spanish translation.
There's also an extra line at the bottom of the help links that I believe shouldn't be there. Noting that we should not push these changes to prod since we have to get them approved by DOS. |
6675fbe
to
3baa59b
Compare
378a738
to
8ea06b7
Compare
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.
Can you add the Chinese translations for the changes in this PR as well?
@mitchellhenke we did plan on doing the Chinese translations in a separate ticket. |
8f2c6a1
to
8b3cce5
Compare
b952eef
to
d882633
Compare
Hi @KeithNava I saw Allison was assigned as a design reviewer for this but since she isn't on the team anymore, I took her off and assigned myself. Please let me know if you need another design review! |
44016d4
to
4dff95d
Compare
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.
LGTM
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.
@KeithNava thank you for attaching the screenshots! I noticed a few things that may have gotten caught in between all the merges:
- Spanish: The body copy (doc_auth_info.how_to_verify) is missing "en una oficina de correos participante." at the end. What is currently implemented is also missing a period at the end, so let's please make sure that is there as well!
- Spanish: The body copy for the verify at a Post Office (doc_auth.info.verify_at_post_office_instruction) is also missing "participante." at the end so it should read " Ingresará la información de su identificación en línea, y verificará su identidad en persona en una oficina de correos participante."
Everything else LGTM!
@rutvigupta-design Thanks for the review - looks like I had a bad merge conflict. Here's the updated screenshot for the Spanish translations. |
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.
This LGTM now! 🎉
@mitchellhenke @gina-yamada I think the merge is awaiting your approval. Thanks! |
config/locales/es.yml
Outdated
doc_auth.info.verify_at_post_office_instruction_selfie: Ingresará la información de su identificación en línea, y verificará su identidad en persona en una oficina de correos participante. | ||
doc_auth.info.verify_at_post_office_link_text: Obtenga más información sobre la verificación en persona | ||
doc_auth.info.verify_at_post_office_link_text: Obtenga más información sobre la verificación en persona. |
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.
@KeithNava I am still seeing inconsistencies with punctuation for both doc_auth.info.verify_online_link_text
and doc_auth.info.verify_at_post_office_link_text
. Translation docs look like all should have a period. See links in the screenshots below. (I am not sure if those where the ones previously added in some case that I suggested to delete. It looks like the translations doc has been cleaned up since so let's use the latest translation doc as the expectations and add them.)
- English needs a period for
doc_auth.info.verify_at_post_office_link_text
anddoc_auth.info.verify_online_link_text
- Chinese needs a period for
doc_auth.info.verify_online_link_text
- French and Spanish are looking 👀 good! 💯
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.
@gina-yamada thanks for pointing this out! I double-checked our designs, and looks like the English links don't have periods at the end, so I think the translations we got back might have missed this context and I copied them into our translations doc. @KeithNava I think we'll want to ensure none of the links have periods at the end – I'll make sure our documentation reflects this. Apologies for all the confusion, wires are getting a bit crossed between all the versions of things.
doc_auth.info.verify_identity: 我们会要求获得你的个人信息并通过与公共记录核对来验证你的身份。 | ||
doc_auth.info.verify_online_description: 如果你没有移动设备或者无法轻松拍身份证件照片,这一选项更好。 | ||
doc_auth.info.verify_online_description_selfie: 如果你有手机可以拍照,请选择该选项。 | ||
doc_auth.info.verify_online_instruction: 你将拍自己身份证件的照片来完全在网上验证身份。大多数用户都能轻松完成这一流程。 | ||
doc_auth.info.verify_online_instruction: 你将拍身份证件的照片来完全在网上验证身份。大多数用户都能轻松完成这样流程。 |
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.
@KeithNava Double check doc_auth.info.verify_online_instruction. I can find the previous translation in the translation doc but not this one.
@@ -113,8 +113,6 @@ class BaseTask | |||
{ key: 'errors.messages.blank_cert_element_req', locales: %i[zh] }, | |||
{ key: 'event_types.sign_in_notification_timeframe_expired', locales: %i[zh] }, | |||
{ key: 'event_types.sign_in_unsuccessful_2fa', locales: %i[zh] }, | |||
{ key: 'forms.buttons.continue_ipp', locales: %i[zh] }, | |||
{ key: 'forms.buttons.continue_remote', locales: %i[zh] }, |
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.
Good clean up!
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.
There are two items you will need to fix; punctuation and 1 Chinese translation. I will approve now so you are not waiting for approval after fixing each.
…lations for Opt-in IPP non-biometric
…lations for Opt-in IPP non-biometric for Chinese, French, and Spanish
bf21979
to
2ebca51
Compare
@rutvigupta-design @gina-yamada Latest screenshots are below |
@KeithNava latest screenshots look great to me! 🎉 |
🎫 Ticket
Link to the relevant ticket:
LG-13046
🛠 Summary of changes
Translation updates for How to Verify Page
📜 Testing Plan
Provide a checklist of steps to confirm the changes.