Add table of SMS/Voice support dynamically loaded from IDP by zachmargolis · Pull Request #707 · GSA-TTS/identity-site

zachmargolis · 2021-10-19T22:27:21Z

This is an idea for a help page with content loaded from the IDP, see IDP PR 18F/identity-idp#5523

If we wanted to launch this:

write and translate the real content
style it differently if needed?
confirm that JS is ok to load this, instead of some build process that loads in the static build step
- (in my mind we're looking to build more dynamic enabling/disabling, so I think that a build step would get stale more quickly, and this is a simple implementation in the IDP so I don't think it would create a big burden)
tests??

Here's how it looks right now:

zachmargolis · 2021-10-19T23:44:47Z

@nickttng suggested we check out the stacked width for mobile, and some visual indicators so I pushed a small change, at least using borderless for now

desktop	mobile

Danielle-Lee · 2021-10-20T14:08:48Z

I think it's a good idea and could fit somewhere in the help content. I'd be curious about accessibility particularly for screen readers. Do we need a visual indicator for both "yes" and "no"? it looks crowded.

zachmargolis · 2021-10-20T14:39:48Z

I think it's a good idea and could fit somewhere in the help content. I'd be curious about accessibility particularly for screen readers. Do we need a visual indicator for both "yes" and "no"? it looks crowded.

The visual indicator was just one idea. Another was having a more sentence-like structure "supports voice and SMS". And we can also choose to hide some content (such as the visual indicators) from screen readers if we are concerned about accessibility.

aduth

Personally I think it'd be marginally better to render this server-side to improve scrapability/indexing and no-JS user support. Maybe with a daily CircleCI job to make sure we're sync'd up. I do think the current implementation here would be much simpler to maintain though, and avoid any delay / overhead in the sync process.

assets/js/country_support.js

_includes/sign_in.html

_includes/country_support_table.html

zachmargolis · 2021-10-20T19:15:10Z

Personally I think it'd be marginally better to render this server-side to improve scrapability/indexing and no-JS user support. Maybe with a daily CircleCI job to make sure we're sync'd up. I do think the current implementation here would be much simpler to maintain though, and avoid any delay / overhead in the sync process.

Yeah I agree it would be great to have that, but AFAICT, Federalist does't have a notion of scheduled builds? So maybe we'd need to give CircleCI write access to GH so it could push to a deploy branch or something?

aduth · 2021-10-20T19:35:13Z

Yeah I agree it would be great to have that, but AFAICT, Federalist does't have a notion of scheduled builds? So maybe we'd need to give CircleCI write access to GH so it could push to a deploy branch or something?

In my mind I pictured something more like what we have with daily sitemap checks or Pinpoint configuration alerts we send to Slack, which still requires effort on part of the developer to push up the pull request to sync the changes.

zachmargolis · 2021-10-20T19:41:43Z

the backend is deploy to DEV and I updated the preview branch to pull from there, so the preview works now: https://federalist-17bd62cc-77b7-4687-9c62-39b462ce6fd5.app.cloud.gov/preview/18f/identity-site/margolis-country-support/help/manage-your-account/international-phone-support/

_sass/components/_sticky-table.scss

assets/js/country_support.js

aduth · 2021-10-20T20:29:49Z

tests??

Couple thoughts:

Puppeteer tests could work for this, but a bit heavy. On the other hand, it already exists!
Could adapt parts of our testing infrastructure we use in the IdP, like Mocha, Sinon (to stub window.fetch), JSDOM (for faking DOM), maybe @testing-library/dom

Co-authored-by: Andrew Duthie <aduth@users.noreply.github.com>

Add additional information to country support table

_includes/country_support_table.html

Account for changes in 35ce65b

Because these specs test more than just accessibility

Missed in ac472fe

zachmargolis · 2021-10-28T17:38:44Z

The prod API endpoint has been deployed, this is ready to go!

aduth · 2021-10-28T17:57:23Z

assets/js/country_support.js

+            isoCode,
+            {
+              name,
+              country_code: countryCode,


One thing we may consider in the future is to have the API respond with locale-translated country names? For example, Germany is "Allemagne" in French, but currently we show all country names in English.

Yes definitely! The IDP itself only has English names right now, but that is a great idea. In the meantime we do that the ISO country codes too

Yeah, I bet we're probably showing English names in some of our translated alert texts. Thinking ones like this in particular:

https://github.com/18F/identity-idp/blob/ef4418520046674acb903d13ddb5552d76a36d3d/config/locales/two_factor_authentication/en.yml#L90

I'll plan to confirm and create a ticket as necessary.

For posterity: LG-5321

assets/js/country_support.js

content/_en/help/manage-your-account/international-phone-support.md

assets/js/country_support.js

_sass/components/_svg.scss

aduth · 2021-10-28T18:10:57Z

_sass/_colors.scss

+// identity-style-guide backports
+$error: #e21c3d;
+$success: #18852e;


Sorry, I had a typo in my original comment. error is the correct token. I think it should be available in the older version?

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

zachmargolis · 2021-10-28T19:58:24Z

Confirmed that this works in IE 11! merging

Add table of SMS/Voice support dynamically loaded from IDP

e568af2

zachmargolis requested review from Danielle-Lee, aduth, jmdembe and mdiarra3 October 19, 2021 22:27

zachmargolis mentioned this pull request Oct 19, 2021

Idea: JSON API to serve current SMS/voice support 18F/identity-idp#5523

Merged

zachmargolis requested review from jauschalley and orenyk October 19, 2021 22:29

zachmargolis added 3 commits October 19, 2021 15:30

fix indentation

fd8f7ea

Update table markup

430c712

table style updates

6eb954c

aduth reviewed Oct 20, 2021

View reviewed changes

assets/js/country_support.js Outdated Show resolved Hide resolved

assets/js/country_support.js Outdated Show resolved Hide resolved

Handle translation via partials, data- attributes

54a812d

zachmargolis commented Oct 20, 2021

View reviewed changes

_includes/sign_in.html Outdated Show resolved Hide resolved

Update _includes/sign_in.html

af97694

zachmargolis commented Oct 20, 2021

View reviewed changes

_includes/country_support_table.html Outdated Show resolved Hide resolved

Update _includes/country_support_table.html

b3d947c

Move early abort

96c25c8

zachmargolis added 2 commits October 20, 2021 13:00

Try adding a sticky table, hiding emoji from screen readers

d7571a2

Add placeholder translations

432b8c7

zachmargolis commented Oct 20, 2021

View reviewed changes

_sass/components/_sticky-table.scss Show resolved Hide resolved

aduth reviewed Oct 20, 2021

View reviewed changes

assets/js/country_support.js Outdated Show resolved Hide resolved

Use String#localeCompare

bd26f85

Co-authored-by: Andrew Duthie <aduth@users.noreply.github.com>

zachmargolis and others added 2 commits October 22, 2021 14:55

Pretty format the dialing code

82d11e9

Merge pull request #709 from 18F/margolis-add-dialing-code

5a7d4b0

Add additional information to country support table

zachmargolis commented Oct 22, 2021

View reviewed changes

_includes/country_support_table.html Show resolved Hide resolved

zachmargolis and others added 8 commits October 22, 2021 15:15

Update _includes/country_support_table.html

72fba67

fix camelCase

45212e9

eslint --fix

bd799b9

Rotate some variable names

0b272c8

Juggle some more variable names

da23b25

Fix table selector for country spec

71a36b0

Account for changes in 35ce65b

Rename "accessibility" job to "e2e"

ac472fe

Because these specs test more than just accessibility

Fix CircleCI missing build name

bebb949

Missed in ac472fe

zachmargolis requested a review from aduth October 28, 2021 17:38

zachmargolis removed the do not merge label Oct 28, 2021

Empty commit to kick off a rebuild with new federalist config

4f179bf

aduth approved these changes Oct 28, 2021

View reviewed changes

zachmargolis and others added 10 commits October 28, 2021 11:18

Update assets/js/country_support.js

5afc5bc

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

Update _sass/components/_svg.scss

2d2c160

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

simplify selector with color

0b41650

reorder pages

b68f93b

use color tokens

66d6022

WIP: fetch polyfill

f564b71

Add pretty import name

91d5833

Add useBuiltins

105b01b

corejs too

5ebf732

javascript transpiler shenanigans

ad842e5

zachmargolis merged commit 4006459 into main Oct 28, 2021

zachmargolis deleted the margolis-country-support branch October 28, 2021 20:06

zachmargolis mentioned this pull request Mar 3, 2022

Add article that renders IDP analytics events from JSON (LG-5590) GSA-TTS/identity-handbook#218

Merged

Conversation

zachmargolis commented Oct 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zachmargolis commented Oct 19, 2021

Uh oh!

Danielle-Lee commented Oct 20, 2021

Uh oh!

zachmargolis commented Oct 20, 2021

Uh oh!

aduth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zachmargolis commented Oct 20, 2021

Uh oh!

aduth commented Oct 20, 2021

Uh oh!

zachmargolis commented Oct 20, 2021

Uh oh!

Uh oh!

Uh oh!

aduth commented Oct 20, 2021

Uh oh!

Uh oh!

zachmargolis commented Oct 28, 2021

Uh oh!

aduth Oct 28, 2021

Choose a reason for hiding this comment

Uh oh!

zachmargolis Oct 28, 2021

Choose a reason for hiding this comment

Uh oh!

aduth Oct 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aduth Oct 28, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aduth Oct 28, 2021

Choose a reason for hiding this comment

Uh oh!

zachmargolis commented Oct 28, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zachmargolis commented Oct 19, 2021 •

edited

Loading

aduth Oct 28, 2021 •

edited

Loading