i18n: add the 32 keys the UI asked for and never had - #541
Merged
Conversation
…ne stacking scale Find Duplicates asked for confirmation twice. The dialog raises its own prompt, and the App callback behind `onDeleteFiles` raised a second one through `setConfirmDialog` whenever "confirm before delete" was on. `ConfirmDialog` is mounted ~300 lines earlier in App.tsx than the finder and both sat at `z-50`, so at an equal z-index DOM order decided it: the finder painted over the question it was waiting on, and its full-viewport backdrop swallowed every click aimed at the buttons underneath. Dragging the finder aside revealed the confirm but did not free it — the panel moves, the overlay does not. The promise never settled, so "Delete Selected" span forever, and the only way to answer was to close the window that had asked. Reported in #537 with all five steps. The dialog now owns the confirmation outright: it takes `confirmBeforeDelete` and skips its own prompt when the setting is off, and the callback only deletes. The stacking half is fixed for the whole app rather than for this one pair. `src/utils/modalLayers.ts` holds the scale, and two things the sweep found go with it: - The app-wide confirm had to clear more than `z-50`. Of the twelve `setConfirmDialog` call sites, the overwrite and delete prompts fire from flows started inside the z-9998..10000 tier (AeroSync, the transfer plan, the trash managers), so a confirm below them would have repeated #537 there. - The lock screens sat at z-100 and z-200, under that same tier. `isAppLocked` gates no rendering in App.tsx — the whole tree stays mounted behind the lock overlay — so a dialog left open when the idle probe fired stayed legible on top of the lock screen. Both now sit above everything. Also: the in-dialog confirm asked `duplicates.confirmDelete`, a key present in none of the 47 locale files, so `t()` printed the key name at the user instead of a question. It now uses `duplicates.deleteConfirm`, which exists. Pins, all four failing on the code they replace: the sweep walks every `fixed inset-0` overlay in the source and refuses one that can cover the confirm or the lock screens; the App mount site must carry no `setConfirmDialog`; the dialog must branch on `confirmBeforeDelete`; and every `t()` key the dialog uses must resolve in en.json, the fallback for all 47 locales. Closes #537 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The #537 sweep started from one string: the Find Duplicates confirmation asked `t('duplicates.confirmDelete')`, a key in none of the 47 locale files, so `t()` did what it does on a miss and printed the key name at the user. Asking the same question of the whole source found 31 more, every one of them reachable — a column header in the B2 hidden-file manager, the buttons in the GitHub branch selector, the version browser's whole vocabulary, the "Later" on an update prompt. The `t('x') || 'Fallback'` idiom at most of those call sites was not protecting anything: `t()` returns the key, the key is truthy, so the fallback never ran. One of them had been carrying an Italian string as the English fallback for exactly that reason, which is how long it had gone unread. Fifteen of the keys name a string the app already had, translated and reviewed, under a different key — `common.path` next to `browser.path`, `errors.unknown` next to `transfer.errorKind.unknown`. Those take the existing translation in each of the 47 locales rather than a new one. The remaining seventeen are new strings and are translated in all 46 locales; `i18n:validate` reports 46/46 clean, no missing keys and no placeholders. The dead `|| '...'` fallbacks are gone from the 13 call sites that had them. `noPhantomKeys.test.ts` asks it of the whole source from now on: every literal `t('a.b')` must resolve in en.json — the file every other locale falls back to before `t()` gives up — and must resolve to a string rather than to a branch of the tree, which would render as "[object Object]". It fails on the tree it replaces with all 32 listed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 14 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR centralizes modal z-index values, moves duplicate deletion confirmation into the dialog, removes inline translation fallbacks, adds missing locale keys across supported languages, and introduces tests for translation keys, deletion behavior, and modal stacking. ChangesUI consistency and localization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant DuplicateFinderDialog
participant ConfirmDialog
participant FileSystem
App->>DuplicateFinderDialog: pass deletion configuration
DuplicateFinderDialog->>ConfirmDialog: show confirmation when enabled
ConfirmDialog-->>DuplicateFinderDialog: confirm deletion
DuplicateFinderDialog->>FileSystem: delete selected paths
FileSystem-->>DuplicateFinderDialog: return deleted paths
DuplicateFinderDialog->>DuplicateFinderDialog: reconcile groups and selections
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
…he snapshot Review finding on #540. `runDelete` closes over `groups` and `selectedPaths` as they were when the button was pressed, then writes them back after awaiting the delete. Deleting many files takes long enough for something to move underneath it: switching mode re-runs the scan, and the checkboxes stay live throughout. The completion would then write the captured groups over newer scan results and clear ticks the user had made after pressing. Both writes are now functional updates keyed by the set of paths that were actually deleted, so a newer scan survives and only what was deleted is unticked. The three controls that start a scan, the two mode toggles and the fuzzy-cutoff field, are disabled while a delete is in flight, which closes the other half of the race rather than only tidying up after it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nto fix/i18n-phantom-keys
Member
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/i18n/locales/fi.json (1)
3390-3390: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the incorrect SSE exclusion.
Line [3390] says SSE-KMS and SSE-C ETags are not MD5 values, but
etag_to_md5accepts any single-part ETag with exactly 32 hexadecimal characters without checking the server-side encryption type. Rewrite this as an advisory, shape-based interpretation.Proposed wording
- "s3-etag": "ETag on objektin MD5 vain yksiosaisille objekteille, joita ei ole salattu SSE-KMS- tai SSE-C-salauksella. Moniosaisen ETagin pääte on `-N`, ja se jätetään pois sen sijaan että arvailtaisiin. SSE-KMS- tai SSE-C-ETag voi silti olla 32 heksadesimaalimerkkiä ja läpäisee tällöin saman muototarkistuksen kuin aito tiiviste, joten raportoitu ETag on tällaiselle objektille vain ohjeellinen.", + "s3-etag": "Täsmälleen 32 heksadesimaalimerkkiä sisältävä yksiosainen ETag voidaan tulkita vain ohjeellisesti MD5-muotoiseksi. Tarkistus ei arvioi palvelinpuolen salauksen tyyppiä, joten tämä voi koskea myös SSE-KMS- ja SSE-C-ETageja. Moniosaiset ETagit, joiden pääte on `-N`, jätetään pois.",Based on learnings,
etag_to_md5treats 32-heksadesimaaliset ETag-arvot MD5-muotoisina riippumatta SSE-KMS- tai SSE-C-salauksesta.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/i18n/locales/fi.json` at line 3390, Update the Finnish “s3-etag” translation to remove the claim that SSE-KMS or SSE-C encryption excludes MD5 interpretation. Describe the behavior as advisory and shape-based: single-part ETags with exactly 32 hexadecimal characters are treated as MD5-shaped regardless of server-side encryption, while multipart ETags ending in “-N” are excluded rather than guessed.Source: Learnings
🧹 Nitpick comments (5)
src/utils/modalLayerSweep.test.ts (2)
22-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse
modalZIndexOf, and drop the unused eslint directive.
readZduplicatesmodalZIndexOffromsrc/utils/modalLayers.ts, which the sibling test already imports. Import it here so the parser cannot drift from the one the source uses. The@typescript-eslint/no-explicit-anydisable on line 22 also has noanyto suppress; the statement casts toRecord<string, string>.♻️ Proposed change
-// eslint-disable-next-line `@typescript-eslint/no-explicit-any` const overlayModules = import.meta.glob('../**/*.tsx', { query: '?raw', import: 'default', eager: true, }) as Record<string, string>; - -/** `z-50` / `z-[9999]` → 50 / 9999. Anything else → null. */ -const readZ = (token: string): number | null => { - const arbitrary = token.match(/^z-\[(\d+)\]$/); - if (arbitrary) return Number(arbitrary[1]); - const scale = token.match(/^z-(\d+)$/); - return scale ? Number(scale[1]) : null; -};Then update the import on line 5:
-import { MODAL_LAYER, MODAL_Z } from './modalLayers'; +import { MODAL_LAYER, MODAL_Z, modalZIndexOf } from './modalLayers';and replace the
readZ(literal[0])call withmodalZIndexOf(literal[0]).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/modalLayerSweep.test.ts` around lines 22 - 35, Remove the unused `@typescript-eslint/no-explicit-any` disable directive and delete the duplicate readZ parser. Import and reuse modalZIndexOf from src/utils/modalLayers.ts, replacing the readZ(literal[0]) call while preserving the existing z-index scanning behavior.
51-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the sweep assertions inside a test, and tighten the z-token match.
Two concerns in
collectOverlays:
- Lines 62 and 72 call
expectat module scope, because line 79 runs the collection during file evaluation. A failure there aborts collection of the whole file instead of failing one named test, so the reported error does not identify the offending overlay as a test result. Collect the overlays lazily inside anit, or throw anErrorwith the same message and keep theexpectcalls in tests.- Line 67 takes the first
z-token in a five-line window that starts two lines before thefixed inset-0match. That token can belong to a neighbouring element rather than to the overlay itself, which records a wrong z-index for the overlay. Prefer matching the z token inside the sameclassNamevalue as thefixed inset-0occurrence.Also note that
windowon line 57 shadows the globalwindowbinding.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/modalLayerSweep.test.ts` around lines 51 - 79, Move the collectOverlays invocation and its expect-based validation into a named test so failures are reported as test failures, and rename the local window variable to avoid shadowing the global binding. In collectOverlays, associate the z-index token with the same className value containing fixed inset-0 rather than searching the broader five-line window, while preserving MODAL_Z handling and the missing-z assertion.src/utils/modalLayers.test.ts (1)
7-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd the elevated-tier ordering assertions.
The doc comment in
src/utils/modalLayers.tsstates two more rules that this suite does not pin:elevatedConfirmmust exceedelevatedModal, andglobalConfirmmust exceed both. A reordering of those three numbers passes today.♻️ Proposed additional assertions
it("puts a modal's own confirm above that modal", () => { expect(MODAL_LAYER.modalConfirm).toBeGreaterThan(MODAL_LAYER.modal); + expect(MODAL_LAYER.elevatedConfirm).toBeGreaterThan(MODAL_LAYER.elevatedModal); + expect(MODAL_LAYER.globalConfirm).toBeGreaterThan(MODAL_LAYER.elevatedConfirm); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/modalLayers.test.ts` around lines 7 - 18, Extend the “modal stacking scale (`#537`)” tests to assert the elevated-tier ordering documented in modal layer constants: verify MODAL_LAYER.elevatedConfirm exceeds MODAL_LAYER.elevatedModal, and MODAL_LAYER.globalConfirm exceeds both elevatedModal and elevatedConfirm. Keep the existing modal and modalConfirm assertions unchanged.src/components/DuplicateFinderDialog.tsx (1)
278-314: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider reconciling after a partial delete failure.
onDeleteFilescan reject after it deletes some paths. In that case thecatchbranch only setserror, sogroupsandselectedPathskeep every path, including the deleted ones. The user then sees rows that no longer exist until a re-scan. A re-scan on failure, or a per-path result fromonDeleteFiles, would close that gap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/DuplicateFinderDialog.tsx` around lines 278 - 314, Update runDelete so partial failures from onDeleteFiles also reconcile successfully deleted paths instead of leaving stale groups and selections. Prefer using per-path deletion results if available; otherwise trigger a re-scan in the catch path before or alongside setting the error, while preserving the existing full-success reconciliation and error message behavior.src/components/DuplicateFinderDialog.test.ts (1)
79-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer behavioral assertions over exact source-text matches.
Line 86 asserts the literal dependency array
}, [selectedPaths, onDeleteFiles]), and line 92 asserts exactly three occurrences ofdisabled={isScanning || isDeleting}. Both fail on a correct change: adding one dependency, reordering it, reformatting the file, or disabling a fourth control. Rendering the dialog with a slowonDeleteFilesand asserting the resulting groups and selection would test the same invariant without coupling to formatting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/DuplicateFinderDialog.test.ts` around lines 79 - 94, Replace the source-text assertions in DuplicateFinderDialog.test.ts with behavioral tests for the delete flow and scan controls. Render the dialog with a delayed onDeleteFiles, verify deleting preserves the correct groups and selected paths, and verify all scan-triggering controls are disabled while deletion is pending; avoid asserting dependency-array text or an exact control count.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/i18n/locales/fi.json`:
- Line 3390: Update the Finnish “s3-etag” translation to remove the claim that
SSE-KMS or SSE-C encryption excludes MD5 interpretation. Describe the behavior
as advisory and shape-based: single-part ETags with exactly 32 hexadecimal
characters are treated as MD5-shaped regardless of server-side encryption, while
multipart ETags ending in “-N” are excluded rather than guessed.
---
Minor comments:
In `@src/components/Sync/VersionBrowser.tsx`:
- Around line 180-182: Update the cleanup result rendering near cleanupResult
and the cloud.cleanupDone translation so “files” and “freed,” along with
deleted_count and freed_bytes formatting, are localized through the translation
system. Use translation interpolation or separate translation keys while
preserving the existing cleanup result condition and formatSize value.
In `@src/i18n/locales/bg.json`:
- Line 4486: Translate the branchOptional value in the Bulgarian locale from
English into Bulgarian, matching the terminology and style used by the adjacent
GitHub branch labels.
- Line 1451: Update the Bulgarian transferComplete translation to use
count-neutral wording or the locale’s pluralization mechanism, ensuring the
message is grammatically correct when count is 1 and for larger counts.
In `@src/i18n/locales/bn.json`:
- Line 4486: Update the branchOptional entry in the Bengali locale to use the
Bengali translation “ব্রাঞ্চ (ঐচ্ছিক)” instead of the English text, preserving
the existing JSON structure and key.
In `@src/i18n/locales/ca.json`:
- Around line 4505-4509: Update the Catalan locale entry github.branchOptional
from the English text to the Catalan translation “Branca (opcional)”, leaving
the surrounding entries unchanged.
In `@src/i18n/locales/cs.json`:
- Line 4486: Update the github.branchOptional translation in cs.json from the
English label to “Větev (volitelná)”, preserving the existing key and JSON
structure.
- Around line 74-83: Restore the missing Czech diacritics in the locale labels:
update the "apply" value to use "Použít", the "saved" value to use "Uloženo",
and the label at the additional location to use "Načítání...".
In `@src/i18n/locales/cy.json`:
- Line 4505: Update the branchOptional entry in the Welsh locale catalog from
the English text to its Welsh translation, using “Cangen (dewisol)” or an
equivalent Welsh wording.
- Line 1298: Update the Welsh translation value for importLegacyHint to replace
“cydweddu cefn” with the approved term “cydnawsedd yn ôl,” preserving the rest
of the sentence, and have a Welsh reviewer verify the final wording.
In `@src/i18n/locales/da.json`:
- Line 4486: Update the branchOptional translation in the Danish locale catalog
from English to the approved Danish wording, using “Gren (valgfri)” unless an
established translation convention specifies otherwise.
In `@src/i18n/locales/de.json`:
- Around line 4486-4490: Update the German locale entry identified by the
working key to use a complete status label such as “In Arbeit” or “Wird
bearbeitet” instead of the incomplete verb “arbeitet”.
In `@src/i18n/locales/el.json`:
- Line 4486: Update the branchOptional locale entry in el.json from the English
label to its Greek translation, preserving the existing JSON structure and key.
- Around line 1448-1451: Update the Greek `transferComplete` translation to use
transfer wording rather than implying files were copied, while preserving the
`{count}` interpolation placeholder; use wording equivalent to “{count} files
were transferred.”
In `@src/i18n/locales/es.json`:
- Line 5345: Update the defaultSaltValueLabel translation to use the Spanish
feminine wording “Sal predeterminada pública”, replacing the English “Salt” and
masculine agreement while preserving the rest of the label.
- Line 1451: Update the Spanish transferComplete translation and its usage in
App.tsx to support singular and plural forms based on summary.transferredFiles,
ensuring one file renders “1 archivo copiado” and multiple files render the
existing plural wording. Do not rely on plain {count} interpolation alone; use
the project’s established pluralization mechanism or separate translation keys.
In `@src/i18n/locales/et.json`:
- Line 4486: Update the github.branchOptional entry in the Estonian locale to
use the translation “Haru (valikuline)” instead of the English text.
In `@src/i18n/locales/eu.json`:
- Line 4505: Update the github.branchOptional locale value in the Basque
translations from the English text to a Basque translation, such as “Adarra
(aukerakoa)”, while preserving the existing key and JSON structure.
In `@src/i18n/locales/fi.json`:
- Line 4486: Update the branchOptional locale entry to use the Finnish
translation “Haara (valinnainen)” instead of the English label, preserving the
existing localization key and JSON structure.
In `@src/i18n/locales/fr.json`:
- Line 2712: Update the toolsNotSupported translation in the French locale to
use the established translated AI settings label “Paramètres IA” instead of
“Paramètres AI”, preserving the rest of the message unchanged.
- Line 4486: Update the branchOptional translation in the French locale to use
feminine gender agreement, replacing “optionnel” with “optionnelle” or
“facultative” while preserving the existing label meaning.
- Around line 74-83: Update the common.saved translation in the French locale
from “Enregistre” to “Enregistré”, leaving the surrounding common labels
unchanged.
- Line 5345: Update the defaultSaltValueLabel translation in the French locale
to use “Sel” instead of “Salt”, preserving the rest of the existing French label
and its meaning.
In `@src/i18n/locales/gl.json`:
- Line 4505: Update src/i18n/locales/gl.json at lines 4505-4505 to translate
branchOptional as “Rama (opcional)”, and at lines 5364-5364 replace the English
“Salt” value with the established Galician term “Sal”.
- Line 4508: Update the rawUrlCopied translation value to use the existing
feminine gender for “URL,” changing “copiado” to “copiada” while preserving the
rest of the Spanish text.
In `@src/i18n/locales/hi.json`:
- Around line 499-502: Update the dropboxPurgeBusinessOnly translation in the
Hindi locale to use “स्थायी रूप से हटाने और ट्रैश खाली करने के लिए” instead of
the grammatically incorrect phrasing, preserving the rest of the message.
- Around line 4486-4490: Update the branchOptional translation in the Hindi
locale so the optional branch label is written in Hindi, matching the
surrounding branchList and noBranches entries while preserving the existing
localization key.
In `@src/i18n/locales/hr.json`:
- Line 2712: Update the Croatian translation value for toolsNotSupported to use
the grammatically correct phrase “u AI postavkama” instead of “u AI postavke,”
preserving the rest of the message unchanged.
- Line 4486: Update the branchOptional entry in the Croatian locale catalog from
the English label to the requested translation, “Grana (neobavezno)”.
In `@src/i18n/locales/hu.json`:
- Line 4486: Translate the branchOptional entry in the Hungarian locale catalog
from the English label into natural Hungarian, preserving the meaning that the
branch selection is optional.
- Line 1298: Update the Hungarian importLegacyHint translation to insert the
definite article before “meglévő”, changing the phrase to “Importálja a
meglévő...” while preserving the rest of the translation.
In `@src/i18n/locales/hy.json`:
- Line 4486: Update the branchOptional locale value in the Armenian translations
from English to Armenian, using the requested meaning “Ճյուղ (ըստ ցանկության)”
while preserving the existing translation key and JSON structure.
In `@src/i18n/locales/id.json`:
- Around line 4486-4490: Update the Indonesian locale entries
github.branchOptional and github.working to use polished Indonesian labels:
“Cabang (opsional)” for the optional branch label and “Berjalan” for the working
status.
In `@src/i18n/locales/is.json`:
- Line 4505: Update the github.branchOptional and github.rawUrlCopied entries in
the Icelandic locale to use natural Icelandic translations, matching neighboring
branch labels and correcting the gender agreement for the feminine noun vefslóð
instead of using “Hrátt”.
In `@src/i18n/locales/ka.json`:
- Line 4486: Update the `github.branchOptional` value in the Georgian locale
from English to a Georgian translation, matching the terminology used by
existing branch labels in the same locale.
In `@src/i18n/locales/km.json`:
- Line 75: Update the `common.add` translation in the Khmer locale from the
incorrect value to `"បន្ថែម"`, matching the existing add labels in the file.
- Around line 4505-4507: Update the branchOptional, branchList, and noBranches
locale entries to use Khmer Git-branch terminology consistently: translate
branchOptional into Khmer and replace មែកធាង with សាខា in the other two values,
preserving the existing JSON keys and structure.
- Line 1451: Update the Khmer toast.transferComplete translation to use the
established ផ្ទេរ term for generic transfer operations instead of ចម្លង; if the
key is strictly copy-only, use the appropriate operation-specific key instead.
In `@src/i18n/locales/lt.json`:
- Line 4486: Translate the branchOptional value in the Lithuanian locale from
the English text to the appropriate Lithuanian wording, preserving the existing
JSON key and valid locale-file syntax.
- Around line 2712-2713: Update the Lithuanian `toolsNotSupported` translation
to use a grammatically complete phrase that clearly identifies `Modeliai` as a
section within AI settings, while preserving the existing instruction to disable
tools.
- Line 5345: Update the Lithuanian defaultSaltValueLabel translation to replace
the dangling “SHA-256 nuo” wording with a complete label that clearly introduces
the displayed digest.
In `@src/i18n/locales/lv.json`:
- Line 1451: Update the Latvian toast.transferComplete translation to use
transfer-neutral wording rather than wording that specifically means “copied,”
while preserving the {count} placeholder and the existing pluralized file-count
format.
- Line 5345: The Latvian defaultSaltValueLabel translation should explicitly
identify the displayed value as a SHA-256 hash. Update only this locale string
to replace the ambiguous “šādas virknes SHA-256” wording with precise Latvian
wording meaning “SHA-256 hash of the following string,” preserving the rest of
the label.
- Line 4486: Translate the github.branchOptional entry in the Latvian locale
catalog, replacing the English “Branch (optional)” value with the appropriate
Latvian translation while preserving the existing JSON key and structure.
- Line 2712: Update the Latvian translation value for toolsNotSupported so the
instruction is grammatically complete and clearly identifies the “AI iestatījumi
→ Modeļi” settings location, while preserving the existing meaning about
disabling “Rīki” for this model.
In `@src/i18n/locales/mk.json`:
- Line 4505: Update the branchOptional translation in the Macedonian locale from
the English text to the Macedonian value “Гранка (опционално)”.
In `@src/i18n/locales/ms.json`:
- Around line 4486-4487: Update the Malay locale entries branchOptional and
branchList to use Malay translations and standardize the GitHub branch
terminology on “cawangan,” matching the existing related labels.
In `@src/i18n/locales/nl.json`:
- Line 4486: Update the Dutch catalog entry identified by the branchOptional key
to replace the English “optional” label with the Dutch translation “optioneel”,
preserving the existing key and formatting.
In `@src/i18n/locales/no.json`:
- Line 4505: Update the branchOptional entry in the Norwegian locale catalog to
use the existing Norwegian term “Gren” while preserving the optional-label
meaning.
- Line 1298: Update the importLegacyHint translation to use the grammatically
correct adjective “enkle” with “AeroCloud-konfigurasjon,” leaving the rest of
the message unchanged.
In `@src/i18n/locales/pl.json`:
- Line 4486: Update the github.branchOptional translation in the Polish locale
from the English label to “Branch (opcjonalnie)”, preserving the existing key
and JSON structure.
- Line 5627: Update the Polish invalidCharacters translation under the
invalidCharacters key to use consistent punctuation after the backslash entry
and explicitly identify the null character as NUL, while preserving the existing
validation meaning.
In `@src/i18n/locales/pt.json`:
- Around line 74-83: Update the save-state translation in the locale entries
around “saved” and “saving” to use consistent European Portuguese: replace
“Salvo” with “Guardado” while retaining “A guardar...” and the surrounding
locale variant.
In `@src/i18n/locales/ro.json`:
- Line 4486: Update the branchOptional translation in the Romanian locale from
the English label to “Ramură (opțional)”, preserving the existing key and JSON
structure.
- Line 1298: Update the Romanian importLegacyHint translation to replace the
literal phrase “păstrează compatibilitatea înapoi” with idiomatic Romanian
wording that clearly conveys preserving backward compatibility, while keeping
the rest of the message unchanged.
- Around line 500-502: Update the Romanian privacy action translations in
setAsPrivate, setAsPublic, and makeAllPrivate to use the same form of address,
replacing the informal imperative in makeAllPrivate with the existing
formal/plural imperative style.
In `@src/i18n/locales/ru.json`:
- Line 1451: Update the transferComplete translation to use the generic wording
“Передано файлов: {count}” instead of wording specific to copying.
- Line 2712: Update the toolsNotSupported translation in the Russian locale to
use grammatically correct wording and the established Russian AI label,
referring users to «в разделе «Настройки ИИ → Модели»» while preserving the rest
of the message.
In `@src/i18n/locales/sk.json`:
- Line 2712: Update the toolsNotSupported translation in the Slovak locale to
use “v Nastaveniach AI → Modely” instead of “v Nastavenie AI → Modely”,
preserving the rest of the message unchanged.
- Line 76: Update the affected Slovak locale labels in the added entries: use
Aplikovať, Odobrať, Uložené, Načítavanie..., and Obnoviť túto verziu, preserving
the existing keys and JSON structure.
- Line 4486: Update the branchOptional translation in the Slovak locale to use
the established GitHub branch terminology, changing it from the English label to
“Vetva (voliteľná)”.
In `@src/i18n/locales/sl.json`:
- Line 4486: Update the `branchOptional` entry in the Slovenian locale to use
the provided translation `Veja (neobvezno)` instead of the English text.
- Line 2712: Update the Slovenian translation value for toolsNotSupported in the
locale data, replacing the ungrammatical navigation phrase with “v nastavitvah
AI → Modeli” while preserving the rest of the message and escaping.
In `@src/i18n/locales/sr.json`:
- Line 2712: Update the toolsNotSupported translation in the Serbian locale to
use “у AI подешавањима” instead of “у AI подешавања”, leaving the rest of the
message unchanged.
- Line 4486: Update the `branchOptional` entry in the Serbian locale to use the
translation `Грана (опционо)` instead of the English label.
- Line 1451: Update the Serbian translation value for transferComplete to use
the generic transfer wording “Пренето {count} датотека” rather than the
copy-specific wording, preserving the existing count placeholder.
In `@src/i18n/locales/sv.json`:
- Line 4486: Update the branchOptional translation in the Swedish locale from
the English label to “Gren (valfritt)”.
In `@src/i18n/locales/sw.json`:
- Line 4486: Update the github.branchOptional translation in the Swahili locale
to use “Tawi (si lazima)” instead of the English text, consistent with the
existing “Tawi” branch translations.
- Line 2712: Update the toolsNotSupported translation to use the corrected
Swahili wording “modeli hii haiwezi kutumia zana,” while preserving the existing
instruction about disabling “Zana” in AI Settings.
In `@src/i18n/locales/th.json`:
- Line 4486: Update the github.branchOptional translation in the Thai locale to
"สาขา (ไม่บังคับ)", while leaving the nearby github.branchSelector translation
unchanged.
In `@src/i18n/locales/tl.json`:
- Line 1450: Update the navigationFailed translation in the tl locale to use
navigation wording rather than transfer/movement wording, preserving the {error}
placeholder and failure message structure.
In `@src/i18n/locales/tr.json`:
- Line 5627: Update the Turkish invalidCharacters translation to explicitly say
“NUL karakteri” instead of the ambiguous “boş karakter,” while preserving the
existing restrictions for slash, backslash, and “..”.
- Line 2713: Update the noProjectContext translation in the Turkish locale to
use “Ajan belleği için proje bağlamı yok.”, replacing the current “Aracı
belleği” wording while preserving the existing key and JSON structure.
- Line 4486: Update the branchOptional translation in the Turkish locale from
the English label to “Dal (isteğe bağlı)”, leaving the surrounding locale
entries unchanged.
- Line 500: Update the Turkish setAsPrivate translation to use “Özel Olarak
Ayarla,” matching the existing private-visibility term while reserving “Gizli”
for hidden visibility.
In `@src/i18n/locales/uk.json`:
- Line 4486: Update the branchOptional translation in the Ukrainian locale to
"Гілка (необов'язково)" instead of the remaining English label.
In `@src/i18n/locales/vi.json`:
- Line 4486: Update the github.branchOptional translation in the Vietnamese
locale from the English label to the Vietnamese value “Nhánh (tùy chọn)”.
---
Nitpick comments:
In `@src/components/DuplicateFinderDialog.test.ts`:
- Around line 79-94: Replace the source-text assertions in
DuplicateFinderDialog.test.ts with behavioral tests for the delete flow and scan
controls. Render the dialog with a delayed onDeleteFiles, verify deleting
preserves the correct groups and selected paths, and verify all scan-triggering
controls are disabled while deletion is pending; avoid asserting
dependency-array text or an exact control count.
In `@src/components/DuplicateFinderDialog.tsx`:
- Around line 278-314: Update runDelete so partial failures from onDeleteFiles
also reconcile successfully deleted paths instead of leaving stale groups and
selections. Prefer using per-path deletion results if available; otherwise
trigger a re-scan in the catch path before or alongside setting the error, while
preserving the existing full-success reconciliation and error message behavior.
In `@src/utils/modalLayers.test.ts`:
- Around line 7-18: Extend the “modal stacking scale (`#537`)” tests to assert the
elevated-tier ordering documented in modal layer constants: verify
MODAL_LAYER.elevatedConfirm exceeds MODAL_LAYER.elevatedModal, and
MODAL_LAYER.globalConfirm exceeds both elevatedModal and elevatedConfirm. Keep
the existing modal and modalConfirm assertions unchanged.
In `@src/utils/modalLayerSweep.test.ts`:
- Around line 22-35: Remove the unused `@typescript-eslint/no-explicit-any`
disable directive and delete the duplicate readZ parser. Import and reuse
modalZIndexOf from src/utils/modalLayers.ts, replacing the readZ(literal[0])
call while preserving the existing z-index scanning behavior.
- Around line 51-79: Move the collectOverlays invocation and its expect-based
validation into a named test so failures are reported as test failures, and
rename the local window variable to avoid shadowing the global binding. In
collectOverlays, associate the z-index token with the same className value
containing fixed inset-0 rather than searching the broader five-line window,
while preserving MODAL_Z handling and the missing-z assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b29df0a-0b35-4114-882c-3aaaf798bcb5
📒 Files selected for processing (70)
src/App.tsxsrc/components/AccountLockScreen.tsxsrc/components/CloudPairsEditor.tsxsrc/components/CloudPanel.tsxsrc/components/CryptomatorCreateDialog.tsxsrc/components/DevTools/AIChat.tsxsrc/components/Dialogs/index.tsxsrc/components/DuplicateFinderDialog.test.tssrc/components/DuplicateFinderDialog.tsxsrc/components/GitHubBranchSelector.tsxsrc/components/GitHubCommitDialog.tsxsrc/components/GitHubPagesBrowser.tsxsrc/components/GitHubWriteModeIndicator.tsxsrc/components/GuardedCloseConfirm.tsxsrc/components/LockScreen.tsxsrc/components/Sync/SelectiveSyncTree.tsxsrc/components/Sync/VersionBrowser.tsxsrc/components/settings/FileAssociationsPanel.tsxsrc/components/settings/SettingsAeroCloudTab.tsxsrc/i18n/locales/bg.jsonsrc/i18n/locales/bn.jsonsrc/i18n/locales/ca.jsonsrc/i18n/locales/cs.jsonsrc/i18n/locales/cy.jsonsrc/i18n/locales/da.jsonsrc/i18n/locales/de.jsonsrc/i18n/locales/el.jsonsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/et.jsonsrc/i18n/locales/eu.jsonsrc/i18n/locales/fi.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/gl.jsonsrc/i18n/locales/hi.jsonsrc/i18n/locales/hr.jsonsrc/i18n/locales/hu.jsonsrc/i18n/locales/hy.jsonsrc/i18n/locales/id.jsonsrc/i18n/locales/is.jsonsrc/i18n/locales/it.jsonsrc/i18n/locales/ja.jsonsrc/i18n/locales/ka.jsonsrc/i18n/locales/km.jsonsrc/i18n/locales/ko.jsonsrc/i18n/locales/lt.jsonsrc/i18n/locales/lv.jsonsrc/i18n/locales/mk.jsonsrc/i18n/locales/ms.jsonsrc/i18n/locales/nl.jsonsrc/i18n/locales/no.jsonsrc/i18n/locales/pl.jsonsrc/i18n/locales/pt.jsonsrc/i18n/locales/ro.jsonsrc/i18n/locales/ru.jsonsrc/i18n/locales/sk.jsonsrc/i18n/locales/sl.jsonsrc/i18n/locales/sr.jsonsrc/i18n/locales/sv.jsonsrc/i18n/locales/sw.jsonsrc/i18n/locales/th.jsonsrc/i18n/locales/tl.jsonsrc/i18n/locales/tr.jsonsrc/i18n/locales/uk.jsonsrc/i18n/locales/vi.jsonsrc/i18n/locales/zh.jsonsrc/i18n/noPhantomKeys.test.tssrc/utils/modalLayerSweep.test.tssrc/utils/modalLayers.test.tssrc/utils/modalLayers.ts
…erlay's own className Four review findings from #541, all in code this PR introduced. `onDeleteFiles` walks the list, so a reject can arrive with earlier paths already deleted. Which ones is not knowable from the dialog, and keeping the old rows left it offering to delete files that no longer exist. It re-scans, which is the only answer that cannot be wrong. The overlay sweep read the first `z-` token in a five-line window that began two lines before the `fixed inset-0` match, so a class belonging to a neighbouring element could be recorded as the overlay's. It now reads the `className` value that actually contains the match, whether that is a quoted string or a `{...}` expression. Making it strict immediately produced a false positive worth keeping out: `SaveAllMenu` has a comment explaining why its confirm is portalled, and the words `fixed inset-0` inside it were being counted as an overlay with no z-index. Comments are blanked before the scan, newlines preserved so the reported line numbers still point at the right place. The sweep also ran at module scope, so a parse failure aborted the file instead of failing a named test; it is collected lazily inside the tests now, and the "every overlay has a z-index" check is one of them rather than a bare `expect` during evaluation. `readZ` duplicated the exported `modalZIndexOf`, which is the parser the source itself uses, and an eslint-disable had no `any` to suppress. The scale gains the assertions that were missing: the elevated tier ordered internally and against the modal tier, and no two layers sharing a number, since an accidental tie is exactly what #537 was. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
Four of the five applied in c81119d (on the #540 branch this stacks on, since those files originate there). The fifth is declined with a reason. Partial delete failure. Valid and now fixed: The z-token window. Valid, and stricter parsing paid for itself immediately. Reading the Module-scope Elevated-tier ordering. Added, plus one the suggestion did not ask for: no two layers may share a number. An accidental tie is precisely what #537 was.
Behavioral assertions over source text. Same answer as on #543: the repo has no DOM harness (vitest on |
…nto fix/i18n-phantom-keys
axpnet
added a commit
that referenced
this pull request
Aug 1, 2026
Second review finding on the same component. `ConfirmOverlay` declares `aria-modal` but only moved focus in: Tab walked straight out of it into the panel underneath, which is still there and still interactive, since these overlays render inside the modal that raised them and hide nothing. On close, focus went to the body, so a keyboard user's next tab run started from the top of the page rather than from the control they had just answered for. Tab and Shift+Tab now wrap between the two buttons, and the element that had focus when the question opened gets it back when the question goes away. The pin from the previous commit was anchored on a fixed 400-character window, which this change pushed the dependency array past. Re-anchored on the effect's own closing array: an assertion that fails because the file grew is an assertion that will be deleted the first time it cries wolf. Declined from the same review: rendering the component and dispatching a real Escape. The repo has no DOM harness at all (vitest runs on `environment: node`, with neither jsdom nor testing-library), so this would be a dependency and lockfile change rather than a test, and the established convention here is the source scan. Same answer already given on #541 and #543. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacks on #540 — please merge that one first.
What the sweep found
#540 started from one string: the Find Duplicates confirmation asked for
t('duplicates.confirmDelete'), a key that existed in none of the 47 locale files, sot()did what it does on a miss and printed the key name at the user instead of a question. Asking the same question of the whole source turned up 31 more, and every one of them is reachable — a column header in the B2 hidden-file manager, all four labels in the GitHub branch selector, the version browser's entire vocabulary, the "Later" on an update prompt.The
t('x') || 'Fallback'idiom at most of those call sites was not protecting anything:t()returns the key, the key is truthy, so the fallback was dead code. One of them had been carrying the Italian string'Più tardi'as the English fallback — which is a fair measure of how long nobody had read that line.What this does
Fifteen of the keys name a string the app already had, translated and reviewed, under a different key:
common.pathbesidebrowser.path,errors.unknownbesidetransfer.errorKind.unknown,toast.transferFailedbesideaeroShare.receive.failedTitle. Those take the existing translation from each of the 47 locales — no new wording, nothing invented.The remaining seventeen are genuinely new strings, and they are translated in all 46 locales.
npm run i18n:validatereports 46/46 clean: no missing keys, no orphans, no[NEEDS TRANSLATION]placeholders. The dead|| '...'fallbacks are removed from the 13 call sites that carried them.The pin
src/i18n/noPhantomKeys.test.tsasks it of the whole source from here on: every literalt('a.b')must resolve inen.json— the file every other locale falls back to beforet()gives up and returns the key — and must resolve to a string rather than to a branch of the tree, which would render as[object Object].Verified failing on the tree it replaces, listing all 32.
Summary by CodeRabbit