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.

Sign up for GitHub

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

Jump to bottom

Chore: Older Adult selection label copy #1513

Merged
merged 7 commits into from
Jul 11, 2023
Merged

Chore: Older Adult selection label copy #1513

merged 7 commits into from
Jul 11, 2023

Conversation

angela-tran
Copy link
Member

Part of #1473 (updates Older Adult radio button copy)

Also took this opportunity to normalize the naming of msgids and filenames related to selection labels (tried to keep it minimal and complete 🙏 ).

@angela-tran angela-tran added this to the Veterans milestone Jul 10, 2023
@angela-tran angela-tran requested a review from a team as a code owner July 10, 2023 23:21
@angela-tran angela-tran self-assigned this Jul 10, 2023
@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 migrations [auto] Review for potential model changes/needed data migrations updates tests Related to automated testing (unit, UI, integration, etc.) front-end HTML/CSS/JavaScript and Django templates and removed migrations [auto] Review for potential model changes/needed data migrations updates labels Jul 10, 2023
thekaveman
thekaveman reviewed Jul 10, 2023
View reviewed changes
benefits/core/migrations/0002_data.py Outdated Show resolved Hide resolved
benefits/core/migrations/0002_data.py Show resolved Hide resolved
benefits/core/migrations/0002_data.py Show resolved Hide resolved
benefits/locale/en/LC_MESSAGES/django.po Outdated Show resolved Hide resolved
benefits/locale/es/LC_MESSAGES/django.po Outdated Show resolved Hide resolved
@thekaveman
Copy link
Member

thekaveman commented Jul 10, 2023 •
edited
Loading

I also think this should wait for #1506 because it touches the same PO strings.

Nevermind.

@thekaveman
Copy link
Member

thekaveman commented Jul 11, 2023 •
edited
Loading

I kind of think we should either make this PR close #1472 or else save these changes for that issue, this is getting further away from the modal.

I'm editing #1472 so it only focuses on the text outside of the radio list, which I think is the distinction being made in this PR.

@thekaveman thekaveman changed the title Chore: selection label copy Chore: Older Adult selection label copy Jul 11, 2023
@machikoyasuda machikoyasuda self-requested a review July 11, 2023 16:17
@angela-tran angela-tran force-pushed the chore/selection-label-copy branch from 77ad58d to 8f0c2d9 Compare July 11, 2023 16:34
@angela-tran angela-tran requested a review from thekaveman July 11, 2023 16:44
@angela-tran
Copy link
Member Author

angela-tran commented Jul 11, 2023 •
edited
Loading

Would it have been better for this PR to just rename the msgids and file name for the senior flow? Really I just want the Older Adult radio button text to be updated so I can put the Login.gov modal trigger in there.

The question I ran into though was whether I should just rename things to senior/selection-label--senior.html and leave the others as selection_label___X or go ahead and finish out the rename for all selection labels.

@angela-tran
Copy link
Member Author

I could definitely just drop the last 4 commits in this PR so that this doesn't touch the other flows

thekaveman
thekaveman approved these changes Jul 11, 2023
View reviewed changes
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 is fine, would rather just get it in so we can keep moving.

@angela-tran angela-tran merged commit de0484d into dev Jul 11, 2023
@angela-tran angela-tran deleted the chore/selection-label-copy branch July 11, 2023 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thekaveman thekaveman thekaveman approved these changes

@machikoyasuda machikoyasuda Awaiting requested review from machikoyasuda

Assignees

@angela-tran angela-tran

Labels
back-end Django views, sessions, middleware, models, migrations etc. deployment-dev [auto] Changes that will trigger a deploy if merged to dev front-end HTML/CSS/JavaScript and Django templates i18n Copy: Language files or Django i18n framework tests Related to automated testing (unit, UI, integration, etc.)
Projects
No open projects
Benefit eligibility verification for veterans using transit in CA
Status: Done
Milestone
Veteran rider enrollment pathway
Development

Successfully merging this pull request may close these issues.
3 participants
@angela-tran @thekaveman @machikoyasuda