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

Feat: eligibility confirm copy and UX #2026

Merged
merged 9 commits into from
Apr 25, 2024

Conversation

angela-tran
Copy link
Member

@angela-tran angela-tran commented Apr 15, 2024

Closes #2011

This PR updates the copy as shown in Figma and updates styles for the Eligibility Confirm form fields.

Followed precedent set by #1610 in using unit-less value for line-height

Reviewing

Screenshots

Desktop

Before (using test-benefits) After (this branch) Figma
image image image

Mobile

Before (using test-benefits) After (this branch) Figma
Screen Shot 2024-04-17 at 15 40 37 Screen Shot 2024-04-17 at 15 40 39 image

@angela-tran angela-tran self-assigned this Apr 15, 2024
@github-actions github-actions bot added deployment-dev [auto] Changes that will trigger a deploy if merged to dev back-end Django views, sessions, middleware, models, migrations etc. labels Apr 15, 2024
Copy link

github-actions bot commented Apr 15, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  benefits/eligibility
  forms.py
Project Total  

This report was generated by python-coverage-comment-action

@angela-tran
Copy link
Member Author

angela-tran commented Apr 16, 2024

Hi @indexing and @thekaveman --

I want to confirm that for MST Courtesy Card and SBMTD Reduced Fare Mobility ID, the copy is supposed to be changing from

- This is a 5-digit number on the front and back of your card.
+ This is a 6-digit number on the front of your card.

and

- This is a 4-digit number on the back of your card.
+ This is a 4-digit number on the front of your card.

(also see screenshot on #2011 (comment))

With that confirmed, I can more confidently update our courtesy-card Cypress tests, which are failing because they expect 5 digits, and our sample data in eligibility-server. I hadn't heard of any changes to the format for Courtesy Card or Reduced Fare Mobility ID numbers, so I have uncertainty on where this copy change is coming from.

@angela-tran
Copy link
Member Author

Another question for @indexing (and maybe @srhhnry too):

What should the copy be for the button when it is submitting the form? Currently the CTA goes from "Check eligibility" to "Checking..."



The Figma screen only shows the unsubmitted state with copy "Find my record"

@srhhnry
Copy link

srhhnry commented Apr 16, 2024

Good catch! My inclination is this button becomes "Finding...". However, I'd like to revisit it later. @indexing Thoughts?

@indexing
Copy link
Member

Let's leave it as "Checking..." to keep our scope intact. If we change the button label in the processing state, that copy needs to be represented in Figma so it can be captured in Ditto for translation.

We can address processing state as an aspect of the effort to complete the button component in the new Figma library.

@thekaveman
Copy link
Member

@angela-tran @indexing @srhhnry -- regarding this:

I want to confirm that for MST Courtesy Card and SBMTD Reduced Fare Mobility ID, the copy is supposed to be changing from

- This is a 5-digit number on the front and back of your card.
+ This is a 6-digit number on the front of your card.

and

- This is a 4-digit number on the back of your card.
+ This is a 4-digit number on the front of your card.

I think this is just a mistake for the Courtesy Cards number of digits change. We haven't heard about any change from MST to the format of their Courtesy Cards. They are still 5-digits as far as we know.

@thekaveman
Copy link
Member

@indexing @srhhnry I think we do need an answer on the front and back difference.

Do we know definitively where these numbers are displayed for Courtesy Cards and Reduced Fare Mobility ID cards? (It may be different per agency).

@angela-tran angela-tran force-pushed the feat/eligibility-confirm-copy-ux branch from ea56541 to 63bb251 Compare April 17, 2024 19:43
@angela-tran
Copy link
Member Author

Dropped f856862 and 237b571 based on @thekaveman 's comments.

If it turns out we do need to change the helper text copy for the card number field, I can make new commits

@angela-tran angela-tran force-pushed the feat/eligibility-confirm-copy-ux branch from 63bb251 to f749cc9 Compare April 17, 2024 20:37
@angela-tran angela-tran marked this pull request as ready for review April 17, 2024 20:44
@angela-tran angela-tran requested a review from a team as a code owner April 17, 2024 20:44
@indexing
Copy link
Member

@angela-tran I can confirm that the digit count for both agency cards has not changed.

@angela-tran angela-tran marked this pull request as draft April 18, 2024 14:53
@angela-tran
Copy link
Member Author

I just noticed in my Before vs. After vs. Figma screenshots that there was a change in Figma for the number of columns that the headline and body text should fit within for Desktop.

image

This seems like something we didn't notice / forgot about in design-dev handoff. It seems easy enough and self-contained, so I will go ahead and implement the change.

@angela-tran angela-tran force-pushed the feat/eligibility-confirm-copy-ux branch 2 times, most recently from 45210f6 to c8d3a32 Compare April 19, 2024 20:25
@angela-tran angela-tran marked this pull request as ready for review April 19, 2024 20:48
@angela-tran angela-tran added the i18n Copy: Language files or Django i18n framework label Apr 24, 2024
thekaveman
thekaveman previously approved these changes Apr 25, 2024
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.

This looks great to me!

@angela-tran
Copy link
Member Author

Sorry @thekaveman , I had to rebase and resolve a conflict with the timestamp in our django.po files

@angela-tran angela-tran merged commit ea4407b into dev Apr 25, 2024
13 checks passed
@angela-tran angela-tran deleted the feat/eligibility-confirm-copy-ux branch April 25, 2024 17:28
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.

Eligibility confirm: make copy more specific, and improve field helper text
4 participants