Skip to content

LG-11139: question upon exit#9392

Merged
dawei-nava merged 17 commits intomainfrom
feature/LG-11139-question-upon-exit
Oct 25, 2023
Merged

LG-11139: question upon exit#9392
dawei-nava merged 17 commits intomainfrom
feature/LG-11139-question-upon-exit

Conversation

@dawei-nava
Copy link
Contributor

@dawei-nava dawei-nava commented Oct 16, 2023

🎫 Ticket

LG-11139: Add optional question about other ID types

🛠 Summary of changes

Add optional survey questions to document image capturing page.
Record user submission as analytics events.
Redirect user to SP specific or general account page when user chooses Exit or "Submit and exit".

📜 Testing Plan

Provide a checklist of steps to confirm the changes.
With Service Provider:

  • Step 1: Navigate to document capturing page, with Service Provider
  • Step 2: Verify the new content displayed on the page
  • Step 3: Click exit link, verify landing to SP failure to proof page
  • Step 4: Repeat and choose ID types, click on button
  • Step 5: Verify landing to SP failure to proof page and analytics events logged
    Without Service Provider:
  • Step 1: Navigate to document capturing page, with Service Provider
  • Step 2: Verify the new content displayed on the page
  • Step 3: Click exit link, verify landing to cancellation(confirmation) page
  • Step 4: Repeat and choose ID types, click on button
  • Step 5: Verify landing to cancellation(confirmation) page and analytics events logged

Sample event:

{
  "name":"Frontend: IdV: exit optional questions",
  "properties":{
    "event_properties":{
      "ids":[
        {
          "name":"us_passport",
          "checked":false
        },
        {
          "name":"resident_card",
          "checked":false
        },
        {
          "name":"military_id",
          "checked":false
        },
        {
          "name":"tribal_id",
          "checked":false
        },
        {
          "name":"voter_registration_card",
          "checked":false
        },
        {
          "name":"other",
          "checked":false
        }
      ],
      "flow_path":"standard",
      "acuant_sdk_upgrade_a_b_testing_enabled":"false",
      "use_alternate_sdk":"false",
      "acuant_version":"11.9.1"
    },
    "new_event":true,
    "path":"/api/logger",
    "session_duration":19.410147,
    "user_id":"3321d388-1fc4-47bc-b7a0-5d59ef52d735",
    "locale":"en",
    "user_ip":"127.0.0.1",
    "hostname":"localhost",
    "pid":33993,
    "service_provider":null,
    "trace_id":null,
    "git_sha":"b2a23958",
    "git_branch":"feature/LG-11139-question-upon-exit",
    "user_agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/117.0.0.0 Safari/537.36",
    "browser_name":"Chrome",
    "browser_version":"117.0.0.0",
    "browser_platform_name":"macOS",
    "browser_platform_version":"10.15.7",
    "browser_device_name":"Unknown",
    "browser_mobile":false,
    "browser_bot":false
  },
  "time":"2023-10-16T18:54:03.411Z",
  "id":"b3f499f4-fed1-41e6-b7ef-6e250c744a36",
  "visitor_id":"fc02aabb-3cb8-4923-af91-b26ac1487b47",
  "visit_id":"ce63dcbb-f9d8-4461-9823-8b2bd11d7942",
  "log_filename":"events.log"
}

👀 Screenshots

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

Without Service Provider:

English:
En-Nosp

Spanish:
Nosp-Es

French:

Nosp-Fr

With Service Provider:

English:
En-Sp

Spanish:
Sp-Es

French:
Sp-Fr

@dawei-nava dawei-nava requested a review from kellular October 16, 2023 19:16
changelog: User-Facing Improvements, IdV document capture, Add optional questions when users decide not to continue
@dawei-nava dawei-nava force-pushed the feature/LG-11139-question-upon-exit branch from 86cbe40 to c1cd972 Compare October 16, 2023 20:03
@dawei-nava dawei-nava requested review from a team and amirbey and removed request for a team October 17, 2023 14:46
@dawei-nava dawei-nava marked this pull request as ready for review October 17, 2023 20:33
@@ -0,0 +1,49 @@
import { render } from '@testing-library/react';
Copy link
Contributor

Choose a reason for hiding this comment

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

Tests look really thorough here. 👍🏻

@eileen-nava
Copy link
Contributor

Since kellular is out, I requested a review from @daviddsilvanava to cover design considerations.

@@ -0,0 +1,21 @@
import { FieldsetHTMLAttributes, ReactNode } from 'react';

export interface FieldSetProps extends FieldsetHTMLAttributes<HTMLFieldSetElement> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Really nice work with these reusable components 👍

Interesting on the difference in capitalization between "Fieldset" and "FieldSet", even between React and TypeScript's built-in DOM typings. I'm not sure which is "correct", though I think I'd agree with you that its spelled-out "field set" as two words would make sense capitalized as you've proposed.

Copy link

@daviddsilvanava daviddsilvanava left a comment

Choose a reason for hiding this comment

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

A couple items to flag:

  1. There seems to be more line padding/spacing around the optional tag and survey question in the screenshot compared to Figma. Is there a way to tighten that up?

  2. In the spanish translation, Tarjeta verde --> Tarjeta Verde

  3. tarjeta de residente permanente --> Tarjeta de Residente Permanente

  4. Carte verte américaine --> Carte Verte américaine

  5. carte de résident permanent --> Carte de Résident Permanent

@dawei-nava
Copy link
Contributor Author

A couple items to flag:

  1. There seems to be more line padding/spacing around the optional tag and survey question in the screenshot compared to Figma. Is there a way to tighten that up?
  2. In the spanish translation, Tarjeta verde --> Tarjeta Verde
  3. tarjeta de residente permanente --> Tarjeta de Residente Permanente
  4. Carte verte américaine --> Carte Verte américaine
  5. carte de résident permanent --> Carte de Résident Permanent

@daviddsilvanava , should we update translation doc here https://docs.google.com/document/d/1-1_DNOWj2z6V7vSN18SyhW3zrAkWgdVjPm-lueWJKIA/edit

@daviddsilvanava
Copy link

A couple items to flag:

  1. There seems to be more line padding/spacing around the optional tag and survey question in the screenshot compared to Figma. Is there a way to tighten that up?
  2. In the spanish translation, Tarjeta verde --> Tarjeta Verde
  3. tarjeta de residente permanente --> Tarjeta de Residente Permanente
  4. Carte verte américaine --> Carte Verte américaine
  5. carte de résident permanent --> Carte de Résident Permanent

@daviddsilvanava , should we update translation doc here https://docs.google.com/document/d/1-1_DNOWj2z6V7vSN18SyhW3zrAkWgdVjPm-lueWJKIA/edit

Yes, all set!

@dawei-nava
Copy link
Contributor Author

A couple items to flag:

  1. There seems to be more line padding/spacing around the optional tag and survey question in the screenshot compared to Figma. Is there a way to tighten that up?
  2. In the spanish translation, Tarjeta verde --> Tarjeta Verde
  3. tarjeta de residente permanente --> Tarjeta de Residente Permanente
  4. Carte verte américaine --> Carte Verte américaine
  5. carte de résident permanent --> Carte de Résident Permanent

@daviddsilvanava , should we update translation doc here https://docs.google.com/document/d/1-1_DNOWj2z6V7vSN18SyhW3zrAkWgdVjPm-lueWJKIA/edit

Yes, all set!

@daviddsilvanava Updated screenshots.

@daviddsilvanava
Copy link

A couple items to flag:

  1. There seems to be more line padding/spacing around the optional tag and survey question in the screenshot compared to Figma. Is there a way to tighten that up?
  2. In the spanish translation, Tarjeta verde --> Tarjeta Verde
  3. tarjeta de residente permanente --> Tarjeta de Residente Permanente
  4. Carte verte américaine --> Carte Verte américaine
  5. carte de résident permanent --> Carte de Résident Permanent

@daviddsilvanava , should we update translation doc here https://docs.google.com/document/d/1-1_DNOWj2z6V7vSN18SyhW3zrAkWgdVjPm-lueWJKIA/edit

Yes, all set!

@daviddsilvanava Updated screenshots.

@dawei-nava on the third option in english (military ID), we also need a ")" to close the parentheses.

@dawei-nava
Copy link
Contributor Author

A couple items to flag:

  1. There seems to be more line padding/spacing around the optional tag and survey question in the screenshot compared to Figma. Is there a way to tighten that up?
  2. In the spanish translation, Tarjeta verde --> Tarjeta Verde
  3. tarjeta de residente permanente --> Tarjeta de Residente Permanente
  4. Carte verte américaine --> Carte Verte américaine
  5. carte de résident permanent --> Carte de Résident Permanent

@daviddsilvanava , should we update translation doc here https://docs.google.com/document/d/1-1_DNOWj2z6V7vSN18SyhW3zrAkWgdVjPm-lueWJKIA/edit

Yes, all set!

@daviddsilvanava Updated screenshots.

@dawei-nava on the third option in english (military ID), we also need a ")" to close the parentheses.

@daviddsilvanava , updated, another question should we use "Military ID card" or "Military ID Card"?

@daviddsilvanava
Copy link

daviddsilvanava commented Oct 20, 2023 via email

@daviddsilvanava
Copy link

lower case is fine for Military ID card

On Fri, Oct 20, 2023 at 2:52 PM dawei-nava @.> wrote: A couple items to flag: 1. There seems to be more line padding/spacing around the optional tag and survey question in the screenshot compared to Figma. Is there a way to tighten that up? 2. In the spanish translation, Tarjeta verde --> Tarjeta Verde 3. tarjeta de residente permanente --> Tarjeta de Residente Permanente 4. Carte verte américaine --> Carte Verte américaine 5. carte de résident permanent --> Carte de Résident Permanent @daviddsilvanava https://github.com/daviddsilvanava , should we update translation doc here https://docs.google.com/document/d/1-1_DNOWj2z6V7vSN18SyhW3zrAkWgdVjPm-lueWJKIA/edit Yes, all set! @daviddsilvanava https://github.com/daviddsilvanava Updated screenshots. @dawei-nava https://github.com/dawei-nava on the third option in english (military ID), we also need a ")" to close the parentheses. @daviddsilvanava https://github.com/daviddsilvanava , updated, another question should we use "Military ID card" or "Military ID Card"? — Reply to this email directly, view it on GitHub <#9392 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVQYLCEDTGWKLQUCSS2DOM3YALJARAVCNFSM6AAAAAA6CVNNM2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONZTGMYTKMZYGA . You are receiving this because you were mentioned.Message ID: @.>

@dawei-nava not sure why this came in as a separate comment, but see above

Copy link

@kellular kellular left a comment

Choose a reason for hiding this comment

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

Reviewed the updated screenshots -- this looks great, Dawei!

@eileen-nava eileen-nava self-requested a review October 24, 2023 20:55
Copy link
Contributor

@eileen-nava eileen-nava left a comment

Choose a reason for hiding this comment

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

I tested locally with and without a SP. Looks good. Approved.

@dawei-nava dawei-nava merged commit 43e1303 into main Oct 25, 2023
@dawei-nava dawei-nava deleted the feature/LG-11139-question-upon-exit branch October 25, 2023 12:09
@mdiarra3 mdiarra3 mentioned this pull request Oct 26, 2023
come_back_later: Lettre avec un crochet
legal_statement:
information_collection: >-
Esta recopilación de información cumple con los requisitos del título 44
Copy link
Contributor

@aduth aduth Dec 19, 2023

Choose a reason for hiding this comment

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

@dawei-nava I've been doing an audit of French/Spanish mismatches and stumbled upon this. It appears we have French and Spanish mixed up in these texts. (This text is Spanish)

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, strangely it looks like the paragraph changes to French about halfway through ("Nous estimons ..."). The same happens in the Spanish implementation (changes from French to Spanish at the burden estimation portion ("Calculamos que tomará ...")

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 at #9801

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aduth , thanks, not sure what happened, it was supposed to be copied from translations.

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.

5 participants