Fix: Make device connection text more generic #424
Conversation
WalkthroughThe pull request updates translation strings across several language files (German, English, Spanish, French, Italian, Dutch, Polish, and Portuguese) for the key 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/helpers/strings-de.ts(1 hunks)src/helpers/strings-en.ts(1 hunks)src/helpers/strings-es.ts(1 hunks)src/helpers/strings-fr.ts(1 hunks)src/helpers/strings-it.ts(1 hunks)src/helpers/strings-nl.ts(1 hunks)src/helpers/strings-pl.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/helpers/strings-de.ts
- src/helpers/strings-it.ts
- src/helpers/strings-en.ts
🔇 Additional comments (3)
src/helpers/strings-nl.ts (1)
81-81: LGTM! The Dutch translation is clear and aligns with the objective.The translation is grammatically correct and successfully conveys the generic device message while maintaining a natural flow.
src/helpers/strings-pl.ts (1)
81-81: LGTM! The Polish translation is clear and aligns with the objective.The translation is grammatically correct and successfully conveys the generic device message while maintaining a natural flow.
src/helpers/strings-fr.ts (1)
81-81: LGTM! The translation change aligns with the PR objectives.The updated French translation "Si vous avez connecté votre appareil, revenez plus tard pour consulter vos données." (If you have connected your device, come back later to view your data) successfully:
- Makes the text more generic by removing specific platform references
- Maintains clarity and proper French grammar
- Aligns with the goal of reducing user confusion when specific device types aren't activated
| "external-account-refresh": "Actualizar", | ||
| "external-account-remove": "Eliminar", | ||
| "device-data-no-data": "Si ha conectado Apple Health, Google Fit, Fitbit o Garmin, vuelva más tarde para ver sus datos.", | ||
| "device-data-no-data": "Si ha conectado tu dispositivo, vuelva más tarde para ver sus datos.", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix pronoun agreement in Spanish translation.
The current translation mixes formal and informal pronouns ("ha conectado" with "tu"). For consistency, it should use either:
Informal style (preferred as the rest of the file uses informal "tu"):
- "device-data-no-data": "Si ha conectado tu dispositivo, vuelva más tarde para ver sus datos.",
+ "device-data-no-data": "Si has conectado tu dispositivo, vuelve más tarde para ver tus datos.",Or formal style:
- "device-data-no-data": "Si ha conectado tu dispositivo, vuelva más tarde para ver sus datos.",
+ "device-data-no-data": "Si ha conectado su dispositivo, vuelva más tarde para ver sus datos.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "device-data-no-data": "Si ha conectado tu dispositivo, vuelva más tarde para ver sus datos.", | |
| "device-data-no-data": "Si has conectado tu dispositivo, vuelve más tarde para ver tus datos.", |
There was a problem hiding this comment.
Had to consult LLM on this one.
However, there’s a small inconsistency in the Spanish translation. The original version used a formal tone ("ha conectado") but the updated string switches to the informal possessive ("tu dispositivo"). You might want to adjust it to either use the formal possessive ("su dispositivo") or change the conjugation to match the informal register.
There was a problem hiding this comment.
@bxh You're absolutely right! There's an inconsistency in the Spanish translation where formal and informal registers are mixed in the same sentence. The updated string uses "ha conectado" (formal) with "tu dispositivo" (informal).
To fix this, you can either:
- Use consistent formal register:
"device-data-no-data": "Si ha conectado su dispositivo, vuelva más tarde para ver sus datos.",
- Use consistent informal register:
"device-data-no-data": "Si has conectado tu dispositivo, vuelve más tarde para ver tus datos.",
The informal option (#2) tends to be more common in modern applications and creates a friendlier tone, but either option would resolve the inconsistency.
dylanbox
force-pushed
the
dylan/device-connection-text
branch
from
d8ea2d6 to
f3cd331
Compare
src/helpers/strings-en.ts
Outdated
| "external-account-refresh": "Refresh", | ||
| "external-account-remove": "Remove", | ||
| "device-data-no-data": "If you have connected Apple Health, Google Fit, Fitbit, or Garmin, check back later to view your data.", | ||
| "device-data-no-data": "If you connected your device, check back later to view your data.", |
There was a problem hiding this comment.
I think this reads slightly better with "have connected" as it was before. Several of the other languages still seem to be using "have connected" as well.
dylanbox
force-pushed
the
dylan/device-connection-text
branch
from
f3cd331 to
7626788
Compare
lynnfaraday
left a comment
lynnfaraday
left a comment
There was a problem hiding this comment.
Probably want to update to latest main to get PT-PT in there.
3 participants
Overview
The Default Device Data Tab (in view library 1) has a little blurb about connecting devices. To avoid confusion the text should be made generic so that if a device type isn't turned on for a project, it wouldn't confuse a participant
Issue: https://github.com/CareEvolution/Product/issues/476
Security
REMINDER: All file contents are public.
Describe briefly what security risks you considered, why they don't apply, or how they've been mitigated.
Checklist
Testing
Documentation
Consider "Squash and merge" as needed to keep the commit history reasonable on
main.Reviewers
Assign to the appropriate reviewer(s). Minimally, a second set of eyes is needed ensure no non-public information is published. Consider also including: