Skip to content

LG-7620: Update IPP CTA language to include Baltimore option#7058

Merged
allthesignals merged 7 commits intomainfrom
wmg/7620-ipp-cta-updates
Sep 30, 2022
Merged

LG-7620: Update IPP CTA language to include Baltimore option#7058
allthesignals merged 7 commits intomainfrom
wmg/7620-ipp-cta-updates

Conversation

@allthesignals
Copy link
Contributor

@allthesignals allthesignals commented Sep 29, 2022

Update FR, ES, and EN IPP CTA language to include Baltimore option.

Ran these changes through make normalize_yaml

Follow up should include updates to: https://github.com/18F/identity-site/blob/20e3e81cd7ff94491f8709e4b1d2b52efd18bfa8/content/_help/verify-your-identity/verify-your-identity-in-person._en.md.

Note: This is a continuation of the now-closed PR.

🎫 Ticket

Link to the relevant ticket.

🛠 Summary of changes

Change IPP CTA language to include Baltimore, MD. Note the intentional newline after "or".

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Check that CI passes
  • Visual check after local setup?

👀 Screenshots

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

Before: image
After: image

@allthesignals allthesignals requested a review from aduth September 29, 2022 17:55
@allthesignals allthesignals marked this pull request as ready for review September 29, 2022 17:56
@allthesignals allthesignals deleted the wmg/7620-ipp-cta-updates branch September 29, 2022 18:50
@allthesignals allthesignals restored the wmg/7620-ipp-cta-updates branch September 29, 2022 18:53
@allthesignals allthesignals reopened this Sep 29, 2022
@allthesignals allthesignals force-pushed the wmg/7620-ipp-cta-updates branch from 3cd7431 to c942b49 Compare September 29, 2022 18:54
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

This looks good 👍 with the caveat that we need #7034 to be merged first for this to work as expected.

troubleshooting:
headings:
are_you_near: 'Are you near Washington, D.C.?'
are_you_near: 'Are you located near Washington, D.C. <br/>or Baltimore, MD?'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we check if the line break element here may have some unintended impact on accessibility, such as a pause when reading? From what I recall, it may. We could consider adding an aria-hidden attribute to have it ignored by assistive technology. (This would need to be added as part of the formatHTML mapping, like in the example in the new documentation introduced with #7034).

Copy link
Contributor Author

@allthesignals allthesignals Sep 29, 2022

Choose a reason for hiding this comment

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

Good point, @aduth looping in @svalexander do you know what the effects might be? According to this answer <br/> means "read without pause."

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
@aduth
Copy link
Contributor

aduth commented Sep 30, 2022

For the current build failure, you may want to rebase your branch against the latest main, since the failure there was fixed in #7039.

@allthesignals
Copy link
Contributor Author

@aduth Thanks! I'm going to merge this once #7034 is merged

@aduth
Copy link
Contributor

aduth commented Sep 30, 2022

@aduth Thanks! I'm going to merge this once #7034 is merged

👍 I just merged #7034 . I might suggest taking it for a spin with another rebase but otherwise you should be set.

troubleshooting:
headings:
are_you_near: 'Are you near Washington, D.C.?'
are_you_near: 'Are you located near Washington, D.C. <br/>or Baltimore, MD?'
Copy link
Contributor

@aduth aduth Sep 30, 2022

Choose a reason for hiding this comment

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

Continuing the discussion from the previous thread, another reason for non-breaking spaces vs. explicit newlines can be to avoid situations where variations in screen size or locale can result in scenarios where a line break would need to occur before the explicit line break.

Example in French + iPhone SE:

image

This feels like a bit of an edge case so I'm not especially concerned about it in this instance, more that the general practice of trying to control line breaks like this can be more trouble than it's worth.

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, @aduth. I went ahead and merged, but I want to investigate this some more.

I'm not sure I'm following entirely, but it actually seems like we want some combination of a line break opportunity element and non-breaking spaces:

Are you located near Washington,&nbsp;D.C. <wbr/>or Baltimore,&nbsp;MD?

Something like this would occur:
Sep-30-2022 14-46-24

I think this is a situation where my asking design their input on this would have helped a bit.

I'll open a follow-up PR

@allthesignals allthesignals merged commit b17633c into main Sep 30, 2022
@allthesignals allthesignals deleted the wmg/7620-ipp-cta-updates branch September 30, 2022 19:37
@aduth aduth mentioned this pull request Oct 3, 2022
jskinne3 pushed a commit that referenced this pull request Oct 12, 2022
* Update IPP CTA language to include Baltimore option

Includes updates to FR, ES, and EN locales.

* Use formatHTML helper and br tag to generate newlines

* changelog: Improvements, In-Person Proofing, Include new location in IPP CTA

* Update README.md

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>

Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
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.

2 participants