Skip to content

LG-6970 create inherited proofing basic landing page#6823

Merged
jscodefix merged 8 commits intomainfrom
LG-6970-inherited-proofing-basic-landing-page
Aug 25, 2022
Merged

LG-6970 create inherited proofing basic landing page#6823
jscodefix merged 8 commits intomainfrom
LG-6970-inherited-proofing-basic-landing-page

Conversation

@jscodefix
Copy link
Contributor

The objective is to create a single landing page with minimal code, that used the FSM (v2) methodology.

technical notes on creating the inherited_proofing_controller.rb and use of FSM (flow state machine):

  • user session management is not included
  • it does not (use these before hooks):
    :confirm_two_factor_authenticated, :redirect_if_pending_in_person_enrollment, :redirect_if_pending_profile,
    :extend_timeout_using_meta_refresh_for_select_paths, :redirect_if_flow_completed, :override_document_capture_step_csp,
    :update_if_skipping_upload, :check_for_outage, :override_csp_for_threat_metrix
  • analytics tracking is not included
  • getting_started.html.erb does not include: javascript_packs_tag_once('document-capture-welcome')
  • not feature flagged

@holytoastr holytoastr added the inherited proofing Pull Requests for the Inherited Proofing feature label Aug 23, 2022
@aduth
Copy link
Contributor

aduth commented Aug 23, 2022

not feature flagged

Could you share more context behind this? Personally, I would have expected it to be feature-flagged, since it's not intended to be ready for production usage yet.

Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

LGTM after adding the non-English content 👍

voip_check: true
voip_block: true
voip_allowed_phones: '[]'
inherited_proofing_va_base_url: 'https://staging-api.va.gov'
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for alphabetizing!

Comment on lines +21 to +24
<%= f.button :button,
t('inherited_proofing.buttons.continue'),
type: :submit,
class: 'usa-button--big usa-button--wide' %>
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest using the submit helper here, which bakes in the standard conventions of a submit button:

Suggested change
<%= f.button :button,
t('inherited_proofing.buttons.continue'),
type: :submit,
class: 'usa-button--big usa-button--wide' %>
<%= f.submit t('inherited_proofing.buttons.continue') %>

<%= simple_form_for :inherited_proofing,
url: url_for,
method: 'put',
html: { autocomplete: 'off', class: 'margin-y-5 js-consent-continue-form' } do |f| %>
Copy link
Contributor

Choose a reason for hiding this comment

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

The js-consent-continue-form class is only relevant for the unsupervised proofing "Consent" screen, so I wouldn't expect to see it here.

Suggested change
html: { autocomplete: 'off', class: 'margin-y-5 js-consent-continue-form' } do |f| %>
html: { autocomplete: 'off', class: 'margin-y-5' } do |f| %>

es:
inherited_proofing:
buttons:
continue: replaceme
Copy link
Contributor

Choose a reason for hiding this comment

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

Will we create a ticket to replace these, if one doesn't already exist? Asking since I recall instances in the past where placeholder content persisted longer than we had intended it to.

Copy link
Contributor

@holytoastr holytoastr Aug 24, 2022

Choose a reason for hiding this comment

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

There's already a ticket and it's in this new sprint! ✌️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, there is a follow-on story (in the next sprint) for adding the translations. Our team is figuring out the required stories and ordering. Requiring the translations up-front, is kind-of a pain. Maybe the answer (in this case) is to create the page without any translation content (including English, ugg!).

Copy link
Contributor

@sheldon-b sheldon-b left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Contributor

@holytoastr holytoastr left a comment

Choose a reason for hiding this comment

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

Thanks for walking me through the code the other day!

Copy link
Contributor

@gangelo gangelo left a comment

Choose a reason for hiding this comment

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

Very nice job.

@jscodefix jscodefix merged commit 5b6c456 into main Aug 25, 2022
@jscodefix jscodefix deleted the LG-6970-inherited-proofing-basic-landing-page branch August 25, 2022 14:24
@aduth aduth mentioned this pull request Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

inherited proofing Pull Requests for the Inherited Proofing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants