Skip to content

LG-10330 Make No In Person Locations View Configurable#9230

Merged
gina-yamada merged 10 commits intomainfrom
yamada/LG-10330-MakeNoInPersonLocConfig-latest
Sep 19, 2023
Merged

LG-10330 Make No In Person Locations View Configurable#9230
gina-yamada merged 10 commits intomainfrom
yamada/LG-10330-MakeNoInPersonLocConfig-latest

Conversation

@gina-yamada
Copy link
Contributor

@gina-yamada gina-yamada commented Sep 18, 2023

🎫 Ticket

LG-10330 Make PO Search's no results view configurable

🛠 Summary of changes

  1. Updated AddressSearch and FullAddressSearch to take in an optional prop noInPersonLocationsDisplay else use the default NoInPersonLocationsDisplay from address-search. This component then gets passed into InPersonLocations for each.
  2. InPersonLocations component no longer uses default NoInPersonLocationsDisplay. The component now is passed in as a prop to noInPersonLocationsDisplay
  3. Increased the version number of identity-address-search
  4. Removed the trailing period for in_person_proofing English and Spanish translations for none_found to match the mocks and the French translation

📜 Testing Plan

To test locally and only the regression (not the configurability change).....

  • Step 1 Open application.yml. Ensure in_person_full_address_entry_enabled is set to true
  • Step 2 Open app/services/usps_in_person_proofing/mock/responses/request_facilities_response.json. Delete all object locations inside the postOffices array. Your file should look like this: { "postOffices": [] }
  • Step 3 Start the server. Set up an account, upload an image to fail proofing. Continue through the application
  • Step 4 Once on the Find a Participating Post Office page, fill out the form
  • Step 5 Observe the Sorry, there are no... section (with icon) as mocked out here. This section/view is now configurable and because you are in ipd, nothing should change. The default view is displaying- that is what you are confirming- nothing is broke- it is working as expected (now that it is configurable).
  • Step 7 Repeat some of the steps above to view in Spanish and French
  • Step 7 Restore the /request_facilities_response.json file above. Run through steps again. This time the Sorry, there are no... message should not display. You should observe a list of Post Offices. Select one and move on to the State Address form to prove everything is working as expected and you can move to the next view in the application
  • Step 8 The configurability change is tested in the unit tests. Also, this new configurability will be used in the idp-sites soon too to cover the remainder of the tasks in this issue

👀 Screenshots

Screenshot 2023-09-18 at 1 23 12 PM

Screenshot 2023-09-18 at 1 32 03 PM

Screenshot 2023-09-18 at 1 32 45 PM

none_found: Sorry, there are no participating Post Offices within 50 miles of
%{address}.
%{address}
none_found_tip: You can search using a different address, or add photos of your
Copy link
Contributor Author

@gina-yamada gina-yamada Sep 18, 2023

Choose a reason for hiding this comment

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

The mocks and the French translation for none_found do not have a trailing period so I removed the period for the English and Spanish translations.

Screenshot 2023-09-18 at 1 28 45 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

If it hasn't already, then this verbiage change should get a cursory review by @18F/identity-joy-designers. The outcome may also affect the changes in 18F/identity-site#1167 since I don't see a prior design review for those changes.

Copy link
Contributor Author

@gina-yamada gina-yamada Sep 19, 2023

Choose a reason for hiding this comment

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

@rutvigupta-design Here is the piece that could use a review, please and thank you. Reach out on Slack if you have questions or want to jump on a call.

Choose a reason for hiding this comment

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

@gina-yamada matching the English and Spanish versions to the Figma designs makes sense to me. Screenshots look good to me!

@gina-yamada gina-yamada marked this pull request as ready for review September 18, 2023 19:33
@gina-yamada gina-yamada requested review from a team and svalexander September 18, 2023 22:00
@NavaTim NavaTim requested review from a team and WheresHJ and removed request for a team September 18, 2023 23:05
Copy link
Contributor

@NavaTim NavaTim left a comment

Choose a reason for hiding this comment

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

The verbiage should get a design sign-off before merging.

The testing steps could be a bit clearer. The steps provided look like they only cover regression and not the configurability change. It looks like the configurability is covered by unit tests, and I would like to see that briefly mentioned in the testing section of the PR when the core change won't be covered by regular end-user testing.

It looks like the code changes are alright, though.

none_found: Sorry, there are no participating Post Offices within 50 miles of
%{address}.
%{address}
none_found_tip: You can search using a different address, or add photos of your
Copy link
Contributor

Choose a reason for hiding this comment

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

If it hasn't already, then this verbiage change should get a cursory review by @18F/identity-joy-designers. The outcome may also affect the changes in 18F/identity-site#1167 since I don't see a prior design review for those changes.

Copy link

@rutvigupta-design rutvigupta-design 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 to me!

@gina-yamada gina-yamada merged commit 3eecdbc into main Sep 19, 2023
@gina-yamada gina-yamada deleted the yamada/LG-10330-MakeNoInPersonLocConfig-latest branch September 19, 2023 21:20
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.

3 participants