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

Eligibility Confirm: Update copy, add form input helper text #959

Merged
merged 3 commits into from
Sep 28, 2022

Conversation

machikoyasuda
Copy link
Member

@machikoyasuda machikoyasuda commented Sep 27, 2022

closes #944

What this PR does

  • Adds form_sub_help_text, form_sub_name_text to initial migration, sample data, model and conftest for testing
  • Add copy for mst.fields.sub.help_text and mst.fields.name.help_text in English and TODO for Spanish
  • Updates existing copy for English and Spanish
  • Adds TODO for placeholder, to remind us to update the placeholder for sub form input

When testing this PR

Remember to run ./bin/init.sh

Screenshots

http://localhost:11369/eligibility/confirm

English
image

Spanish
image

@machikoyasuda machikoyasuda added this to the Courtesy Cards milestone Sep 27, 2022
@machikoyasuda machikoyasuda requested a review from a team as a code owner September 27, 2022 00:06
@machikoyasuda machikoyasuda self-assigned this Sep 27, 2022
@github-actions github-actions bot added i18n Copy: Language files or Django i18n framework back-end Django views, sessions, middleware, models, migrations etc. deployment-dev [auto] Changes that will trigger a deploy if merged to dev labels Sep 27, 2022
Copy link
Member

@angela-tran angela-tran left a comment

Choose a reason for hiding this comment

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

These changes look good to me 👍

@@ -146,9 +146,11 @@ def load_sample_data(app, *args, **kwargs):
form_content_title=_("eligibility.pages.confirm.mst.content_title"),
form_blurb=_("eligibility.pages.confirm.mst.p[0]"),
form_sub_label=_("eligibility.forms.confirm.mst.fields.sub"),
form_sub_placeholder="B1234567",
form_sub_help_text=_("eligibility.forms.confirm.mst.fields.sub.help_text"),
form_sub_placeholder="TODO: B1234567",
Copy link
Member

Choose a reason for hiding this comment

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

I think we can make this #12345 based on design discussions

Copy link
Member

Choose a reason for hiding this comment

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

But this raises an interesting question - we don't have the # symbol in the data we'll get from Velocity. So we may need to strip it out here before sending to the Server?

Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

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

LGTM!

@machikoyasuda machikoyasuda merged commit 8503218 into dev Sep 28, 2022
@machikoyasuda machikoyasuda deleted the feat/944-eligibility-confirm--form-helper branch September 28, 2022 00:03
@angela-tran
Copy link
Member

Forgot to note earlier: if we haven't already, we should update data migrations in benefits-secrets to have the new form_sub_help_text and form_name_help_text fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back-end Django views, sessions, middleware, models, migrations etc. deployment-dev [auto] Changes that will trigger a deploy if merged to dev i18n Copy: Language files or Django i18n framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Eligibility Confirm page with new copy
3 participants