Skip to content

Conversation

@philuren66
Copy link
Contributor

@philuren66 philuren66 commented Sep 5, 2025

Revived DanChae's branch; finished adding the translations as per NES-311; fixed more translations that were also missing.

This PR supercedes this one: #3994

Notes For Testers are in NES-311.

Summary by CodeRabbit

  • Style

    • Localized numerous UI labels and tooltips (e.g., None, No Image, See all journeys, Click to edit, Save).
    • Standardized capitalization and corrected icon labels (Message Square, Radio Button Unchecked).
    • Localized alt text for images and updated default image alt.
  • Bug Fixes

    • Prevented potential crash when adding a new image if no children exist.
    • Corrected scripture reference typos to “Deuteronomy 10:11”.
  • Tests

    • Updated test expectations and fixtures to reflect new translations and corrected text.

DanChae03 and others added 23 commits November 5, 2024 04:21
…translated-labels-buttons' into danchae/eng-1293-untranslated-labels-buttons
@philuren66 philuren66 requested a review from jianwei1 September 5, 2025 04:48
@philuren66 philuren66 self-assigned this Sep 5, 2025
@philuren66 philuren66 added the Bug label Sep 5, 2025
@linear
Copy link

linear bot commented Sep 5, 2025

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ philuren66
❌ DanChae03
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 5, 2025

Walkthrough

Localization updates across admin UI: added/adjusted t(...) usage, corrected labels, and updated English locale resources. Fixed “Deuteronomy 10:11” typos in code and tests. Minor UI tweak removed a fixed button width. Added null-safe parentOrder in new image creation. Adjusted tests to match updated UI text.

Changes

Cohort / File(s) Summary
UI localization updates
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/Properties/Accordion/Accordion.tsx, .../Properties/blocks/SignUp/SignUp.tsx, .../Editor/Toolbar/JourneyDetails/JourneyDetailsDialog/JourneyDetailsDialog.tsx, .../Editor/Toolbar/Toolbar.tsx, .../JourneyList/JourneyCard/JourneyCard.tsx, .../Properties/blocks/RadioOption/RadioOption.tsx, .../Properties/blocks/Button/Button.tsx, .../CanvasDetails/AddBlock/NewImageButton/NewImageButton.tsx, .../Drawer/VideoLibrary/VideoFromLocal/VideoFromLocal.tsx
Added/adjusted translated strings (e.g., None, Save, tooltips, alt text); delocalized “Jesus Film Library” to literal; NewImageButton alt uses t('Default Image Icon').
Icon/label text corrections
.../JourneyAppearance/Chat/ChatOption/Details/Details.tsx, .../Properties/controls/Icon/Icon.tsx
Fixed labels: “Message Square” spelling; “Radio Button Unchecked” spelling in icon list.
Scripture caption typo fixes
apps/journeys-admin/src/libs/useJourneyCreateMutation/useJourneyCreateMutation.ts, .../StepBlockNode/StepBlockNode.stories.tsx, .../JourneyList/ActiveJourneyList/AddJourneyButton/AddJourneyButton.spec.tsx, .../OnboardingPanel/OnboardingPanel.spec.tsx, .../libs/useJourneyCreateMutation/useJourneyCreateMutation.spec.tsx
Corrected “Deutoronomy 10:11” → “Deuteronomy 10:11” in defaults, stories, and tests.
Tests updated for UI text
.../Properties/blocks/RadioOption/RadioOption.spec.tsx
Updated expectations to “No Image” capitalization in ARIA queries.
Styling adjustment
apps/journeys-admin/src/components/AccessDialog/AddUserSection/AddUserSection.tsx
Removed fixed button width from sx.
Locale resources (en)
libs/locales/en/apps-journeys-admin.json
Added/renamed keys (e.g., None, No Image, Save, See all journeys, Default Image Icon, Deuteronomy 10:11); corrected spellings; reorganized entries.
Minor logic safety
.../CanvasDetails/AddBlock/NewImageButton/NewImageButton.tsx
parentOrder now uses card.children?.length ?? 0.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Pre-merge checks (2 passed, 3 warnings)

❌ Failed checks (3 warnings)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning The changes address many NES-311 coding objectives: the Save button translation was added (JourneyDetailsDialog.tsx), the Jesus Film Library label was set to literal English (VideoFromLocal.tsx), the Default Image Icon key was applied (NewImageButton.tsx), and numerous UI labels across Accordion, Button, Toolbar, JourneyCard, icons, and the en locale file were updated. However several checklist items from NES-311 remain outstanding in this PR — specifically the Access email-dropdown spacing, analytics tooltip wording, the Card Properties "Dark" style key, and various typography/button chosen-option translations which were deferred to NES-778. Test and story typos were fixed but do not replace the missing localization keys. Overall the PR partially satisfies NES-311 but does not fully complete the linked issue. Please add or update t() calls and locale keys for the Access dropdown and analytics tooltip, ensure the missing "Dark" style key and other style/button keys are added to locales and propagated via Crowdin, and explicitly document which items are deferred to NES-778; include a concise list of remaining keys in the PR description so reviewers and QA can verify coverage.
Out of Scope Changes Check ⚠️ Warning Most edits are i18n-related, but a few changes appear outside NES-311’s stated scope: removal of a fixed button width in AddUserSection.tsx, the parentOrder optional-chaining change in NewImageButton.tsx (logic), and multiple test/story fixture typo fixes; these styling/logic/test changes are not localization tasks and increase PR scope. Additionally the en locale JSON includes many transient additions/removals that should be verified to avoid losing keys in Crowdin. These out-of-scope items are small but make the PR harder to review purely as an i18n change. Recommendation: split non-i18n styling or logic fixes and test/story typo corrections into separate commits or PRs, or at minimum document and justify them in the PR description and add related issue references; also provide a concise changelog of added/removed locale keys to help Crowdin reconciliation.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "fix: add/fix missing translations" is concise and accurately summarizes the PR’s primary purpose of adding and correcting localization across the Admin UI, so it communicates intent to reviewers and history scans. It is short, on-topic, and avoids noise; a minor improvement would be to choose a single verb (e.g., "add" or "fix") or an i18n scope prefix for consistency, but this is optional.
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch philipuren/nes-311-untranslated-labels-buttons

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@nx-cloud
Copy link

nx-cloud bot commented Sep 5, 2025

View your CI Pipeline Execution ↗ for commit 1a42470

Command Status Duration Result
nx run journeys-admin-e2e:e2e ✅ Succeeded 40s View ↗
nx run journeys-e2e:e2e ✅ Succeeded 12s View ↗
nx run watch-modern-e2e:e2e ✅ Succeeded 6s View ↗
nx run watch-e2e:e2e ✅ Succeeded 11s View ↗
nx run videos-admin-e2e:e2e ✅ Succeeded 6s View ↗
nx run-many --target=vercel-alias --projects=vi... ✅ Succeeded 1s View ↗
nx run-many --target=upload-sourcemaps --projec... ✅ Succeeded 6s View ↗
nx run-many --target=deploy --projects=videos-a... ✅ Succeeded 1m 53s View ↗
Additional runs (12) ✅ Succeeded ... View ↗

☁️ Nx Cloud last updated this comment at 2025-09-11 03:01:46 UTC

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/AddBlock/NewImageButton/NewImageButton.tsx (1)

53-53: Nice null-safe parentOrder fix.

Using optional chaining prevents the crash when children is undefined and keeps optimistic ordering correct.

🧹 Nitpick comments (1)
apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/AddBlock/NewImageButton/NewImageButton.tsx (1)

55-55: Don’t persist a placeholder as alt text (a11y).

Persisting t('Default Image Icon') risks shipping that string if editors don’t edit the alt. Prefer creating the block with an empty alt and show the translated placeholder only in the UI.

Suggested change:

-      alt: t('Default Image Icon'),
+      alt: '',
  • If backend requires non-empty alt, fallback to t('Image') instead of “Default Image Icon”.
  • Re: your PR questions:
    • Long keys: stay consistent with existing natural-language keys in this namespace; don’t invent shorter identifiers unless you plan a repo-wide migration.
    • ko-KR additions: prefer Crowdin for missing translations; use manual entries only for temporary QA and sync them through Crowdin afterward to avoid drift.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e0abe90 and 5188ae0.

📒 Files selected for processing (3)
  • apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/AddBlock/NewImageButton/NewImageButton.tsx (1 hunks)
  • apps/journeys-admin/src/libs/useJourneyCreateMutation/useJourneyCreateMutation.ts (1 hunks)
  • libs/locales/en/apps-journeys-admin.json (7 hunks)
✅ Files skipped from review due to trivial changes (1)
  • apps/journeys-admin/src/libs/useJourneyCreateMutation/useJourneyCreateMutation.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • libs/locales/en/apps-journeys-admin.json
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/base.mdc)

**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.

Files:

  • apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/AddBlock/NewImageButton/NewImageButton.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/base.mdc)

Define a type if possible.

Files:

  • apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/AddBlock/NewImageButton/NewImageButton.tsx
apps/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/apps.mdc)

apps/**/*.{js,jsx,ts,tsx}: Always use MUI over HTML elements; avoid using CSS or tags.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.

Files:

  • apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/AddBlock/NewImageButton/NewImageButton.tsx
🧠 Learnings (1)
📚 Learning: 2025-02-04T16:36:58.743Z
Learnt from: Kneesal
PR: JesusFilm/core#5100
File: apps/api-journeys-modern/src/schema/blocks/card/card.zod.ts:0-0
Timestamp: 2025-02-04T16:36:58.743Z
Learning: In the CardBlockSchema (apps/api-journeys-modern/src/schema/blocks/card/card.zod.ts), the parentBlockId field should remain nullable, despite the description indicating it can only be a child of a step block.

Applied to files:

  • apps/journeys-admin/src/components/Editor/Slider/Settings/CanvasDetails/AddBlock/NewImageButton/NewImageButton.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: Deploy Preview (watch-modern, 7594/merge, pull_request, 22)
  • GitHub Check: Deploy Preview (videos-admin, 7594/merge, pull_request, 22)
  • GitHub Check: Deploy Preview (journeys, 7594/merge, pull_request, 22)
  • GitHub Check: Deploy Preview (watch, 7594/merge, pull_request, 22)
  • GitHub Check: Deploy Preview (journeys-admin, 7594/merge, pull_request, 22)
  • GitHub Check: test (22, 3/3)
  • GitHub Check: test (22, 1/3)
  • GitHub Check: build (22)
  • GitHub Check: test (22, 2/3)

@github-actions github-actions bot temporarily deployed to Preview - journeys September 11, 2025 02:56 Inactive
@github-actions github-actions bot temporarily deployed to Preview - journeys-admin September 11, 2025 02:56 Inactive
@github-actions github-actions bot temporarily deployed to Preview - watch-modern September 11, 2025 02:56 Inactive
@github-actions github-actions bot temporarily deployed to Preview - videos-admin September 11, 2025 02:56 Inactive
jianwei1

This comment was marked as resolved.

@tataihono tataihono enabled auto-merge September 11, 2025 04:30
@tataihono tataihono disabled auto-merge September 11, 2025 04:30
@tataihono tataihono merged commit c6b6c36 into main Sep 11, 2025
32 of 33 checks passed
@tataihono tataihono deleted the philipuren/nes-311-untranslated-labels-buttons branch September 11, 2025 04:30
This was referenced Jan 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants