Shorten commuter exclusion option copy - #100211
Conversation
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx bun ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
|
@Krishna2323 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d911d232b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| optionHomeAndOfficeHelp: 'Use member’s home address, work arrangement, and office assignment to calculate commute exclusions.', | ||
| optionFixedDistanceTitle: 'Remove fixed distance', | ||
| optionFixedDistanceHelp: 'Subtracts the same distance from every claim.', | ||
| optionHomeAndOfficeTitle: 'Exclude usual commute', |
There was a problem hiding this comment.
When a workspace has no address, choosing this renamed row triggers the workspaceAddressRequired modal, but its prompt still says the user cannot enable the “calculate by home and office” setting; the corresponding prompts in the other locales retain their old names as well. Since no option now has that name, users receive an error referring to a different setting, so update these prompts alongside the option rename.
Useful? React with 👍 / 👎.
| optionHomeAndOfficeTitle: 'Calculer par domicile et bureau', | ||
| optionHomeAndOfficeHelp: 'Utiliser l’adresse personnelle du membre, son mode de travail et son affectation de bureau pour calculer les exclusions de trajet.', | ||
| optionHomeAndOfficeTitle: 'Exclure le trajet domicile-travail habituel', | ||
| optionHomeAndOfficeHelp: 'Soustrait le trajet domicile-travail habituel de chaque membre en fonction de son domicile, de son mode de travail et de son bureau.', |
There was a problem hiding this comment.
In the French locale this replacement remains truncated: PolicyCommuterExclusionsPage limits alternate text to two lines, and BaseSelectListItem caps its width at 303 px, while this sentence is approximately 754 px wide at the default 13 px Expensify Neue size and therefore requires at least three lines. It is also slightly wider than the text it replaces, so it does not resolve the reported visibility problem for French users.
Useful? React with 👍 / 👎.
| summaryDisabled: 'Kein Ausschluss von Pendelstrecken', | ||
| summaryFixedDistance: ({distance, unit}: {distance: number; unit: string}) => `${distance} ${unit} pro Abrechnung ausschließen`, | ||
| optionDisabledTitle: 'Pendelfahrten nicht ausschließen', | ||
| optionDisabledTitle: 'Pendlerpauschale nicht ausschließen', |
There was a problem hiding this comment.
For German users, Pendlerpauschale denotes the commuter tax allowance rather than the commute distance that this disabled mode stops subtracting. The resulting title describes preserving a tax allowance while both the helper text and the actual behavior concern commute trips, so use a commute term such as Pendelfahrt or Arbeitsweg here.
Useful? React with 👍 / 👎.
|
🚧 JmillsExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
JmillsExpensify
left a comment
JmillsExpensify
left a comment
There was a problem hiding this comment.
Much more legible. Thank you!
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
🚧 Gonals has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/Gonals in version: 9.4.73-0 🚀
|
No help site changes are required for this PRThis is a copy-only change to the Exclude commutes option names and supporting text. There is nothing to update in Three reasons:
One in-app copy mismatch worth fixingNot a docs issue, but this PR renamed the option and left a reference to the old name behind:
The option is now Do not exclude commute (singular). That quoted instruction no longer matches any label in the UI, so a user following it looks for an option that doesn't exist. It also renders When this will need a help articleOnce What I checked
I did not run a UI verification pass, since the option labels are gated behind the @fedirjh, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.4.73-3 🚀
Bundle Size Analysis (Sentry): |
6 participants
Explanation of Change
The commuter exclusion option descriptions are truncated after two lines, preventing users from seeing the full explanation. This shortens the option names and supporting text across all supported languages so the explanations remain visible.
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/676565
PROPOSAL:
Tests
Preconditions: Use an account with commuter exclusion beta enabled
Offline tests
Same as Tests.
QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
MacOS: Chrome / Safari