Repository navigation
Fix sidebar bg: add missing locale entries and portal resync - #2622
Conversation
Add English fallback locale entries for all supported languages on the two new matchTerminalBackground keys. Schedule portal geometry resync when the toggle changes to prevent misaligned portals with withinWindow blend mode.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds localized strings for a "Match Terminal Background" feature across multiple languages and an onChange handler in ContentView.swift that schedules terminal-portal geometry synchronization when the sidebar background-matching setting changes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ContentView
participant SidebarState
participant TerminalWindowPortalRegistry
participant Window(s)
User->>ContentView: toggle sidebarMatchTerminalBackground
ContentView->>SidebarState: read isVisible & blendMode
SidebarState-->>ContentView: visible / blendMode value
alt visible && blendMode == withinWindow
ContentView->>TerminalWindowPortalRegistry: scheduleExternalGeometrySynchronize(for observedWindow or all)
TerminalWindowPortalRegistry->>Window(s): apply geometry synchronization
else
ContentView-->>TerminalWindowPortalRegistry: no scheduling
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
This review could not be run because your cubic account has exceeded the monthly review limit. If you need help restoring access, please contact contact@cubic.dev. |
Greptile SummaryThis PR is a follow-up to #2293, adding English fallback locale entries for all 17 non- Confidence Score: 5/5Safe to merge — no correctness issues found. Both changes are narrow and correct: the onChange guard exactly mirrors the useWithinWindow predicate in contentAndSidebarLayout, and the xcstrings fallbacks use the standard needs_review pattern. No P0/P1 findings. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User toggles Match Terminal Background] --> B{sidebarState.isVisible?}
B -- No --> Z[return - no-op]
B -- Yes --> C{sidebarBlendMode == withinWindow?}
C -- No --> Z
C -- Yes --> D{observedWindow set?}
D -- Yes --> E[scheduleExternalGeometrySynchronize for window]
D -- No --> F[scheduleExternalGeometrySynchronizeForAllWindows]
E --> G[Portals re-aligned]
F --> G
Reviews (1): Last reviewed commit: "fix: add missing locale entries and port..." | Re-trigger Greptile |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Resources/Localizable.xcstrings (1)
83713-83813: Change fallback-state entries to match the catalog convention ("translated").All newly added non-English entries for these two keys are set to
"state": "needs_review", but the established pattern inResources/Localizable.xcstringsis to use"state": "translated"for English-fallback values in lower-confidence locales. This convention keeps localization tooling expectations consistent and prevents spurious missing-translation warnings.Change to apply for all newly added locale entries (lines 83715, 83832, and repeated for each locale)
- "state": "needs_review", + "state": "translated",Also applies to: 83832–83932
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/Localizable.xcstrings` around lines 83713 - 83813, Update all non-English locale entries added for the "Match Terminal Background" string (the JSON blocks where "value": "Match Terminal Background") by changing each "stringUnit"."state" from "needs_review" to "translated" so they follow the repository's English-fallback convention and avoid spurious missing-translation warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Resources/Localizable.xcstrings`:
- Around line 83713-83813: Update all non-English locale entries added for the
"Match Terminal Background" string (the JSON blocks where "value": "Match
Terminal Background") by changing each "stringUnit"."state" from "needs_review"
to "translated" so they follow the repository's English-fallback convention and
avoid spurious missing-translation warnings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a679eb3-aade-4b20-a731-8ad438d2c197
📒 Files selected for processing (2)
Resources/Localizable.xcstringsSources/ContentView.swift
Summary
matchTerminalBackgroundlocalization keys (previously only en+ja were present)Follow-up to #2293, addressing review feedback from Greptile and CodeRabbit.
Test plan
Summary by cubic
Prevents portal misalignment when toggling “Match Terminal Background” by resyncing geometry for the within-window blend mode. Adds English fallback strings for the setting and its tooltip across all locales to remove missing-translation warnings.
Written for commit 65cd8d8. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements
Localization