Skip to content

Jmax/lg 9147 show extra guidance for puerto rico addresses#8088

Merged
jmax-gsa merged 18 commits intomainfrom
jmax/LG-9147-show-extra-guidance-for-puerto-rico-addresses
Apr 18, 2023
Merged

Jmax/lg 9147 show extra guidance for puerto rico addresses#8088
jmax-gsa merged 18 commits intomainfrom
jmax/LG-9147-show-extra-guidance-for-puerto-rico-addresses

Conversation

@jmax-gsa
Copy link
Contributor

@jmax-gsa jmax-gsa commented Mar 28, 2023

🎫 Ticket

LG-9147

🛠 Summary of changes

  • Changed address screen (app/views/idv/address/new.html) to always render Puerto Rico address guidance as hidden text.
  • Added TS event handler to show or hide the address guidance as appropriate (driven by the state dropdown)

📜 Testing Plan

  • Create a new user and begin verifying them. Navigate to the confirmation page, and then to the 'change address' page.
  • Verify that the Puerto Rico address guidance is not visible (the default address in dev will not be in Puerto Rico)
  • Select Puerto Rico from the state dropdown and verify that the guidance appears.
  • Select some other state, and verify that the guidance disappears.

👀 Screenshots

Not-PR
pr

@jmax-gsa jmax-gsa self-assigned this Mar 28, 2023
@jmax-gsa jmax-gsa marked this pull request as ready for review March 28, 2023 18:19
@jmax-gsa jmax-gsa force-pushed the jmax/LG-9147-show-extra-guidance-for-puerto-rico-addresses branch from 81f5cfb to d73c5e2 Compare March 29, 2023 15:42
Copy link
Contributor

@artfulaction artfulaction left a comment

Choose a reason for hiding this comment

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

Screenshot LGTM

Copy link
Contributor

@soniaconnolly soniaconnolly left a comment

Choose a reason for hiding this comment

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

I pulled this down and tried it out. It does show/hide the banner on changing the state field, but not the hint text by each field. I'm also wondering if we still need the AddressPresenter I added if we can do this all on the front end with hidden fields.
Screenshot 2023-04-06 at 8 56 50 AM

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it may not be necessary to add this as a new standalone argument, since ValidatedFieldComponent passes through all additional options to SimpleForm. I think you could do something like this instead:

Suggested change
hint_class: @presenter.hint_class,
hint_html: { class: @presenter.hint_class },

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried this and ran into problems. It's a good idea, but not quite this straightforward.

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you also remove the ValidatedFieldComponent revisions? I could imagine with those as implemented, the hint_html class would be overridden. But reverting those changes should work. #8232

@jmax-gsa jmax-gsa force-pushed the jmax/LG-9147-show-extra-guidance-for-puerto-rico-addresses branch from e22c538 to fa7e86e Compare April 18, 2023 15:09
@jmax-gsa jmax-gsa merged commit df3e2be into main Apr 18, 2023
@jmax-gsa jmax-gsa deleted the jmax/LG-9147-show-extra-guidance-for-puerto-rico-addresses branch April 18, 2023 15:57
jc-gsa pushed a commit that referenced this pull request Apr 19, 2023
The Puerto Rico information blurb and address field hints are present but hidden on initial rendering. The client-side JavaScript shows and hide it as appropriate.
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.

4 participants