Skip to content
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

LG-11082 Add Conditional Text To FullAddressSearch Component #9331

Merged
merged 9 commits into from
Oct 10, 2023

Conversation

gina-yamada
Copy link
Contributor

@gina-yamada gina-yamada commented Oct 6, 2023

🎫 Ticket

LG-11082 Refactor identity-site to use FullAddressSearch

There are no mocks for Full Address Search in the Help Center. In this PR, it was decided to add this additional text. The text will display when the FullAddressSearch component is used in the Help Center but it will not display in identity-ipd (login).

🛠 Summary of changes

  1. If handleLocationSelect is null (as would be the case for the Help Center), display Enter an address to find a Post Office near you. else do not display this text
  2. Added a period to the English and Spanish translation files. (French already has a period.)
  3. Increased the version of @18f/identity-address-search from 3.1.0 to 3.1.1
  4. Added testing to check conditional rendering

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Step 1: This will be the normal ipd view from Login.gov --
    Running the application locally, enter the in-person proofing flow and continue until you are on the Find a participating Post Office view. Confirm that you do not see the text Enter an address to find a Post Office near you. (It would be above the Address input field if it did.)

  • Step 2: This will be the view/flow from the Help Center --
    Edit app/javascript/packages/document-capture/components/in-person-location-full-address-entry-post-office-search-step.tsx. Update the arg for handleLocationSelect to be null (See code snipped below 👇). Confirm that you do see the text Enter an address to find a Post Office near you.

<FullAddressSearch
   registerField={registerField}
   onFoundLocations={setLocationResults}
   disabled={disabledAddressSearch}
   locationsURL={locationsURL}
   handleLocationSelect={null} // <--- CHANGE ME <--- CHANGE ME <--- CHANGE ME
   usStatesTerritories={usStatesTerritories}
/>

👀 Screenshots

IPD View

Screenshot 2023-10-06 at 10 30 18 AM

Help Center View (only what is inside the purple box)

Screenshot 2023-10-06 at 10 29 07 AM

Screenshot 2023-10-06 at 10 29 28 AM

Screenshot 2023-10-06 at 10 29 43 AM

@gina-yamada gina-yamada marked this pull request as ready for review October 6, 2023 17:02
@gina-yamada gina-yamada changed the title LG-11082 Add Additional Text To FullAddressSearch Component LG-11082 Add Additional Conditional Text To FullAddressSearch Component Oct 6, 2023
@gina-yamada gina-yamada changed the title LG-11082 Add Additional Conditional Text To FullAddressSearch Component LG-11082 AddConditional Text To FullAddressSearch Component Oct 6, 2023
@gina-yamada gina-yamada changed the title LG-11082 AddConditional Text To FullAddressSearch Component LG-11082 Add Conditional Text To FullAddressSearch Component Oct 6, 2023
@svalexander
Copy link
Contributor

everything works as expected but just checking - will there be a heading above this component when in the help center?

@gina-yamada
Copy link
Contributor Author

gina-yamada commented Oct 6, 2023

everything works as expected but just checking - will there be a heading above this component when in the help center?

@svalexander Yes! There will be a heading in the Help Center. Look at the after screenshot in this PR to see the heading. Thanks for reviewing and testing. I am going to publish to npm soon. If you have never done this- reach out on Slack if you'd like to jump on a call to see it done.

@aduth
Copy link
Member

aduth commented Oct 10, 2023

If handleLocationSelect is null (as would be the case for the Help Center), display Enter an address to find a Post Office near you. else do not display this text

Is there a meaningful relation between this prop and the resulting text, or are we using it because it's incidentally how the brochure site configured the component, and we want to customize the text for the brochure site?

If it's the latter, would it be better to let that content be handled outside the component itself? I guess the alert placement makes this challenging, but I might imagine at least a separate prop for controlling the "intro" text.

@gina-yamada
Copy link
Contributor Author

gina-yamada commented Oct 10, 2023

If handleLocationSelect is null (as would be the case for the Help Center), display Enter an address to find a Post Office near you. else do not display this text

Is there a meaningful relation between this prop and the resulting text, or are we using it because it's incidentally how the brochure site configured the component, and we want to customize the text for the brochure site?

If it's the latter, would it be better to let that content be handled outside the component itself? I guess the alert placement makes this challenging, but I might imagine at least a separate prop for controlling the "intro" text.

@aduth It is the latter. I thought about adding that content outside the component itself but ultimately decided against it for 2 reasons.

First, we are using the value of handleLocationSelect to conditionally render 5 other elements in that component. I thought it made sense to keep the same pattern and let IDP decide when to show/hide even though this particular text is at the top and can be moved outside of this component.

When handleLocationSelect is null...

  • Text Find a participating Post Office is hidden
  • Text You can verify your identity... is hidden
  • Text Select a Post Office is hidden
  • Select buttons are hidden
  • Info alert is displayed (You must start this process...)
  • ... and with this PR, in FullAddressSearch only, display Enter an address to find...

Additional, this text should only display when using FullAddressSearch, not AddressSearch. Currently, which component to use is being determined in IDP by the in_person_full_address_entry_enabled flag. If we moved this text outside of the FullAddressSearch component and into identity-sites, we would have to add logic to conditionally display this text depending on which component IDP was rendering. We would then be maintain two flags and they would always need to be in-sync- which seems more problematic to maintain than this approach here. Alternatively, rather than conditionally rendering it, we can just add/delete it inside identity-sites based on which component we used. This would require some rework of post_office_search as current we are just rendering the packaged component.

Please let me know your thoughts after hearing this information. I alway appreciate your thoughts and value your feedback! Thank you!

@aduth
Copy link
Member

aduth commented Oct 10, 2023

In that case, maybe it's something to follow-up on separately, if at least this is consistent with the current implementation.

I don't necessarily have an issue with a prop controlling different aspects like that, I just don't know that it's very obvious that the presence or absence of a callback would have such an effect on content.

@gina-yamada gina-yamada merged commit 214b400 into main Oct 10, 2023
2 checks passed
@gina-yamada gina-yamada deleted the yamada/LG-11082-fullAddressSearchAdditionalText branch October 10, 2023 19:08
@gina-yamada
Copy link
Contributor Author

address-search version 3.1.1 published to npm

@jmdembe jmdembe mentioned this pull request Oct 12, 2023
jmdembe added a commit that referenced this pull request Oct 12, 2023
* LG-11082 Add Conditional Text To FullAddressSearch Component (#9331)

* Add conditional text to view

* add new tests

* Add period to display text

* package version increase from 3.1.0 to 3.1.1

* fix linter errors

* changelog: Upcoming feature, USPS Full Address Search, Added conditional logic to display/hide text on the Find a participating Post Office view that will display in Help Center only

* Integrate personal key feature specs into end_to_end_idv feature specs (#9336)

Since feature specs run slowly, it's better to check assertions as part of a single longer spec
rather than restart identity verification over and over. This removes several long-running feature
specs from the test suite.

[skip changelog]

* Update specs to initialize session as HashWithIndifferentAccess (#9347)

changelog: Internal, Automated Testing, Improve accuracy of session stubbing in tests

* Change `<b>` tags to `<strong>` for better accessibility and code consistency (#9349)

* Change `<b>` tags to `<strong>` for better accessibility and code consistency

changelog: User-facing Improvements, Accessibility, Use strong html tag instead of b for emphasis

* Enable RSpec/LeakyConstantDeclaration rubocop (#9348)

* Enable RSpec/LeakyConstantDeclaration rubocop

changelog: Internal, Source code, Enable RSpec rubocop

* Use let instead of defining new class

---------

Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov>

* Sync TypeScript-ESLint versions (#9352)

changelog: Internal, Dependencies, Update dependencies to their latest versions

* LG-10037: display warning banner on gpo welcome back page if number of gpo letter requests exceeded (#9303)

* display warning banner on gpo welcome back page if gpo letter requests are spammed

changelog: User-Facing Improvements, Identity Verification, display warning banner if user has sent max letter requests within a time window

* handle if user has no gpo confirmatio codes

* Update app/views/idv/by_mail/enter_code/index.html.erb

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

* tests for alert banner for spammed gpo requesets

* happy linting

* fix extra space in alert_spam_warning_html i18n

* happy linting

* lintfix i18n

* lint line too long

* js tag removal from alert gpo spam banner spec

* integrate warning alert banner for spammed gpo letter requests into existing tests

* refactor test for gpo spam warning banner

* happy linting

* create before action to remove test order dependency

* happy linting

* define  gpo_verification_enabled in review app

* define  gpo_verification_enabled in review app

---------

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

* Upgrade to Rails 7.1 (#9333)

* fix otp missing translations

* rails 7.1

changelog: Internal, Dependencies, Upgrade to Rails 7.1

* fix untranslated webauthn verification

* LG-10837: Add New Piv Cac Logging for login visited (#9294)

* changelog: Internal Fixes, Authentication LG-10837: Piv Cac Logging fixes

* changelog: Internal, Authentication, Add Login visited for pivcac/change logging names to be uniform

* uniform spec test

* fix naming convention for piv cac

* update rspec

* add previous name

* Add lint check for reasonable asset bundle sizes (#9353)

* Add lint check for reasonable asset bundle sizes

changelog: Internal, Automated Testing, Add test for reasonable asset bundle size

* TEMPORARY: Revert "Fix JavaScript dead code elimination (#9217)"

This reverts commit 0fcc3a7.

* Revert "TEMPORARY: Revert "Fix JavaScript dead code elimination (#9217)""

This reverts commit af166f2.

* Update changelog script to reflect non-security Dependabot usage (#9354)

changelog: Internal, Changelog, Update changelog script to reflect non-security Dependabot usage

* Revert "Upgrade to Rails 7.1 (#9333)" (#9356)

This reverts commit f9a0cd0.

* LG-10812 | Report on all-time user count (#9350)

changelog: Internal, Reporting, Monthly report includes all-time user count

* Reorganize combined invoice report for easier manual runs (#9358)

changelog: Internal, Reporting, Reorganize combined-invoice-supplement-report

* Exclude 'IRS Attempt API: Event metadata' events from log results (#9360)

changelog: Internal, Data Requests, Exclude 'IRS Attempt API: Event metadata' events from log results

* Remove Guardfile, guard dependencies (#9364)

changelog: Internal, Dependencies, Remove unused testing dependencies

* LG-11066 Do not redirect users at the phone step unless they are phone and address rate limited (#9345)

Users are being rate limited and encounting the phone error screen even if they can still verify by mail. This commit changes the rate limit logic to allow users to proceed to the phone step if they can still verify their phone or complete verification by mail.

A side-effect of this change is a bug is fixed where the following situation would exist:

1. A user proofed by mail after exhausting phone attempts
2. The user goes to GPO entry and chooses to cancel and start over
3. The user is redirected to the welcome step to start over
4. The welcome step before action observes the user is phone rate limited and sends the user to the phone errors controller
5. The phone errors controller has a before action to confirm the user has completed the phone errors step; the user has not since in this session so they are redirected to the welcome step
6. Steps 4 and 5 complete until there are too many redirects

[skip changelog]

Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov>

---------

Co-authored-by: gina-yamada <125507397+gina-yamada@users.noreply.github.com>
Co-authored-by: Sonia Connolly <sonia.connolly@gsa.gov>
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov>
Co-authored-by: Amir Reavis-Bey <amir.reavis-bey@gsa.gov>
Co-authored-by: Malick Diarra <malick.diarra@gsa.gov>
Co-authored-by: Matt Wagner <mattwagner@navapbc.com>
Co-authored-by: Jonathan Hooper <jonathan.hooper@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.

None yet

3 participants