fix(i18n): resolve translation gaps + language reactivity

[zachyale]

Description

This PR largely re-implements the translation related changed made by @judelgadoc in #107
That PR was closed as a result of hanging for over 2 weeks with excessive merge conflicts.

Linked Issue: Fixes #103

Changes

• Adds localization to some areas that were previously hard coded
• Added some missing translation strings to the json files
• Adds proper reactive localization to some areas that were not reactive on language change

Summary by CodeRabbit

• New Features

  • Icon picker UI fully localized with English and Spanish translations.
  • Entity type labels exposed for translation (library/shelf/magicShelf/all/unshelved).

• Improvements

  • Sort and filter labels now update immediately when the app language changes.
  • Browsing headers and filter displays reflect active locale dynamically.
  • Language chart shows localized language names based on the current language.

[coderabbitai]

📝 Walkthrough

Walkthrough

Components and templates were updated to use Transloco translations and to subscribe to runtime language changes; translation JSON keys for entity types and an icon picker were added; language-chart now localizes language names via Intl.DisplayNames and reactive lang tracking.

Changes

Cohort / File(s)	Summary
Author & Series language subscriptions frontend/src/app/features/author-browser/components/author-browser/author-browser.component.ts, frontend/src/app/features/series-browser/components/.../series-browser.component.ts	Subscribe to t.langChanges$ (with takeUntilDestroyed/destroyRef) and reassign sortOptions/filterOptions labels on language change so dropdown items update at runtime.
Book browser (template, logic, i18n) frontend/src/app/features/book/components/book-browser/book-browser.component.html, frontend/src/app/features/book/components/book-browser/book-browser.component.ts, frontend/src/i18n/en/book.json, frontend/src/i18n/es/book.json	Template now renders localized entity type labels; added getEntityTypeLabelKey() and switched filter label generation to TranslocoService.translate(); added browser.entityTypes keys in en/es JSON.
Book browser filter label updates frontend/src/app/features/book/components/book-browser/book-browser.component.ts	Added language-change subscription to refresh currentFilterLabel; replaced FilterLabelHelper name lookup with transloco keys for filter-type labels while preserving display value formatting.
Language chart localization frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts	Added activeLang signal from t.langChanges$, updated computed languageStats to depend on it, and enhanced getDisplayName() to use Intl.DisplayNames with fallbacks; added helpers (isLanguageCode, capitalize, getActiveLang) and RxJS/toSignal interop.
Icon picker template and logic i18n frontend/src/app/shared/components/icon-picker/icon-picker-component.html, frontend/src/app/shared/components/icon-picker/icon-picker-component.ts, frontend/src/i18n/en/shared.json, frontend/src/i18n/es/shared.json	Wrapped template in transloco namespace and replaced hardcoded strings with t(...); replaced hardcoded error/toast messages with translated keys and parameters; added iconPicker translation namespace in en/es JSON; updated component imports metadata.

Sequence Diagram(s)

(Skipped — changes are per-component i18n updates and do not introduce a new multi-component sequential flow that benefits from a sequence diagram.)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• perf(ui): decouple book browser rendering pipeline #341 — Modifies BookBrowser component template and TypeScript; likely related to i18n and entity label rendering changes.

Suggested reviewers

• imajes
• balazs-szucs

Poem

🐰
I hopped through strings both near and far,
Replaced each "hardcoded" with a star,
When language turns, the labels sing,
Icons, charts — now everything,
Translation carrots — hop, hooray! 🥕✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title follows the conventional commit format with 'fix' type and a clear, descriptive scope about i18n translation gaps and language reactivity.
Description check	✅ Passed	The PR description includes all required template sections: Description, Linked Issue, and Changes with sufficient detail about the PR's purpose, scope, and implementation approach.
Linked Issues check	✅ Passed	The PR successfully addresses all coding requirements from issue #103: adds localization to hardcoded strings (book-browser, icon-picker, language-chart), implements reactive language updates via langChanges$ subscriptions, and uses Intl.DisplayNames for language labels.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to addressing issue #103: localization additions, translation string imports, and reactive language subscriptions are all aligned with the linked issue requirements.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/i18n-translation-reactivity

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch refactor/i18n-translation-reactivity

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (4)

frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts (1)

158-161: Signal dependency invocation could use a clarifying comment.

The this.activeLang() call exists solely to establish a reactive dependency so the computed re-runs on language changes. Consider adding a brief comment to clarify intent for future maintainers.

 public readonly languageStats = computed(() => {
-  this.activeLang();
+  this.activeLang(); // Re-run when active language changes
   return this.calculateLanguageStats(this.filteredBooks());
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts`
around lines 158 - 161, The computed getter languageStats currently calls
this.activeLang() solely to establish a reactive dependency so the computed
re-runs on language changes; add a brief clarifying comment next to the
this.activeLang() invocation (inside the languageStats computed) stating that
the call is intentional and only meant to subscribe to activeLang changes (so
readers won't think it's a no-op bug), keeping the rest of the logic that
returns calculateLanguageStats(this.filteredBooks()) unchanged.

frontend/src/app/features/author-browser/components/author-browser/author-browser.component.ts (1)

215-249: Consider extracting sortOptions initialization to reduce duplication.

The same sortOptions array is built twice: once at initialization and again in the langChanges$ callback. Extracting this into a helper method would improve maintainability.

♻️ Suggested refactor

+  private buildSortOptions(): SortOption[] {
+    return [
+      {label: this.t.translate('authorBrowser.sort.name'), value: 'name'},
+      {label: this.t.translate('authorBrowser.sort.bookCount'), value: 'book-count'},
+      {label: this.t.translate('authorBrowser.sort.matched'), value: 'matched'},
+      {label: this.t.translate('authorBrowser.sort.recentlyAdded'), value: 'recently-added'},
+      {label: this.t.translate('authorBrowser.sort.recentlyRead'), value: 'recently-read'},
+      {label: this.t.translate('authorBrowser.sort.readingProgress'), value: 'reading-progress'},
+      {label: this.t.translate('authorBrowser.sort.avgRating'), value: 'avg-rating'},
+      {label: this.t.translate('authorBrowser.sort.photo'), value: 'photo'},
+      {label: this.t.translate('authorBrowser.sort.seriesCount'), value: 'series-count'}
+    ];
+  }
+
   ngOnInit(): void {
     this.pageTitle.setPageTitle(this.t.translate('authorBrowser.pageTitle'));
-    this.sortOptions = [
-      {label: this.t.translate('authorBrowser.sort.name'), value: 'name'},
-      {label: this.t.translate('authorBrowser.sort.bookCount'), value: 'book-count'},
-      {label: this.t.translate('authorBrowser.sort.matched'), value: 'matched'},
-      {label: this.t.translate('authorBrowser.sort.recentlyAdded'), value: 'recently-added'},
-      {label: this.t.translate('authorBrowser.sort.recentlyRead'), value: 'recently-read'},
-      {label: this.t.translate('authorBrowser.sort.readingProgress'), value: 'reading-progress'},
-      {label: this.t.translate('authorBrowser.sort.avgRating'), value: 'avg-rating'},
-      {label: this.t.translate('authorBrowser.sort.photo'), value: 'photo'},
-      {label: this.t.translate('authorBrowser.sort.seriesCount'), value: 'series-count'}
-    ];
+    this.sortOptions = this.buildSortOptions();
     // ... existing code ...
     this.t.langChanges$
       .pipe(takeUntilDestroyed(this.destroyRef))
       .subscribe(() => {
-        this.sortOptions = [
-          {label: this.t.translate('authorBrowser.sort.name'), value: 'name'},
-          // ... all labels ...
-        ];
+        this.sortOptions = this.buildSortOptions();
       });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/features/author-browser/components/author-browser/author-browser.component.ts`
around lines 215 - 249, Extract the duplicated array construction into a helper
method (e.g. buildSortOptions()) that returns the translated sortOptions array
using this.t.translate, then replace the two inline arrays with this.sortOptions
= this.buildSortOptions(); call both at initialization and inside the
this.t.langChanges$ subscription; keep the method on the component class so
setupScrollPositionTracking and the existing queryParam handling continue to
work unchanged and ensure the helper returns the same item shape ({label,
value}) as used currently.

frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts (1)

128-162: Same duplication pattern as author-browser—consider extracting builders.

The filterOptions and sortOptions arrays are duplicated between initial assignment and the langChanges$ callback. Consider extracting helper methods for consistency with similar components.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts`
around lines 128 - 162, The component duplicates construction of filterOptions
and sortOptions both on initialization and inside the this.t.langChanges$
subscription; extract two helper methods (e.g., buildFilterOptions() and
buildSortOptions()) that return the translated arrays and call those from the
initial assignments and from the this.t.langChanges$ subscribe callback,
replacing the repeated inline arrays so the translations are centralized and
consistent with other components like author-browser.

frontend/src/app/shared/components/icon-picker/icon-picker-component.ts (1)

236-236: Use the shared error helper here for consistency.

Line 236 still calls this.t.translate(...) directly for an error key; routing this through translateError('noIconsToSave') keeps one consistent error translation path.

Suggested small consistency diff

-      this.batchErrorMessage = this.t.translate('shared.iconPicker.errors.noIconsToSave');
+      this.batchErrorMessage = this.translateError('noIconsToSave');

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/src/app/shared/components/icon-picker/icon-picker-component.ts` at
line 236, Replace the direct translation call that sets this.batchErrorMessage
with the shared error helper: instead of calling
this.t.translate('shared.iconPicker.errors.noIconsToSave') assign via the common
helper function (translateError('noIconsToSave')) so error messages follow the
centralized translation path; update the assignment that sets
this.batchErrorMessage accordingly and ensure you import or reference the
component's translateError helper used elsewhere in this file.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@frontend/src/app/features/author-browser/components/author-browser/author-browser.component.ts`:
- Around line 215-249: Extract the duplicated array construction into a helper
method (e.g. buildSortOptions()) that returns the translated sortOptions array
using this.t.translate, then replace the two inline arrays with this.sortOptions
= this.buildSortOptions(); call both at initialization and inside the
this.t.langChanges$ subscription; keep the method on the component class so
setupScrollPositionTracking and the existing queryParam handling continue to
work unchanged and ensure the helper returns the same item shape ({label,
value}) as used currently.

In
`@frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts`:
- Around line 128-162: The component duplicates construction of filterOptions
and sortOptions both on initialization and inside the this.t.langChanges$
subscription; extract two helper methods (e.g., buildFilterOptions() and
buildSortOptions()) that return the translated arrays and call those from the
initial assignments and from the this.t.langChanges$ subscribe callback,
replacing the repeated inline arrays so the translations are centralized and
consistent with other components like author-browser.

In
`@frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts`:
- Around line 158-161: The computed getter languageStats currently calls
this.activeLang() solely to establish a reactive dependency so the computed
re-runs on language changes; add a brief clarifying comment next to the
this.activeLang() invocation (inside the languageStats computed) stating that
the call is intentional and only meant to subscribe to activeLang changes (so
readers won't think it's a no-op bug), keeping the rest of the logic that
returns calculateLanguageStats(this.filteredBooks()) unchanged.

In `@frontend/src/app/shared/components/icon-picker/icon-picker-component.ts`:
- Line 236: Replace the direct translation call that sets this.batchErrorMessage
with the shared error helper: instead of calling
this.t.translate('shared.iconPicker.errors.noIconsToSave') assign via the common
helper function (translateError('noIconsToSave')) so error messages follow the
centralized translation path; update the assignment that sets
this.batchErrorMessage accordingly and ensure you import or reference the
component's translateError helper used elsewhere in this file.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a7bd8b76-104e-4826-9f2b-2e2ef40187bb

📥 Commits

Reviewing files that changed from the base of the PR and between a86d661 and 6a9e5b7.

📒 Files selected for processing (11)

• frontend/src/app/features/author-browser/components/author-browser/author-browser.component.ts
• frontend/src/app/features/book/components/book-browser/book-browser.component.html
• frontend/src/app/features/book/components/book-browser/book-browser.component.ts
• frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts
• frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts
• frontend/src/app/shared/components/icon-picker/icon-picker-component.html
• frontend/src/app/shared/components/icon-picker/icon-picker-component.ts
• frontend/src/i18n/en/book.json
• frontend/src/i18n/en/shared.json
• frontend/src/i18n/es/book.json
• frontend/src/i18n/es/shared.json

📜 Review details

⏰ 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). (3)

• GitHub Check: Test Suite / Frontend Tests
• GitHub Check: Test Suite / Backend Tests
• GitHub Check: Analyze (java-kotlin)

🧰 Additional context used

📓 Path-based instructions (4)

frontend/src/i18n/**

📄 CodeRabbit inference engine (AGENTS.md)

Put user-facing strings in Transloco files under frontend/src/i18n/

Files:

• frontend/src/i18n/en/book.json
• frontend/src/i18n/es/book.json
• frontend/src/i18n/en/shared.json
• frontend/src/i18n/es/shared.json

frontend/src/**/*.{ts,tsx,html,scss}

📄 CodeRabbit inference engine (AGENTS.md)

Use 2-space indentation in TypeScript, HTML, and SCSS files

Files:

• frontend/src/app/features/book/components/book-browser/book-browser.component.html
• frontend/src/app/features/author-browser/components/author-browser/author-browser.component.ts
• frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts
• frontend/src/app/shared/components/icon-picker/icon-picker-component.html
• frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts
• frontend/src/app/shared/components/icon-picker/icon-picker-component.ts
• frontend/src/app/features/book/components/book-browser/book-browser.component.ts

frontend/src/app/**/*.component.ts

📄 CodeRabbit inference engine (AGENTS.md)

Keep Angular code on standalone components. Do not add NgModules

Files:

• frontend/src/app/features/author-browser/components/author-browser/author-browser.component.ts
• frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts
• frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts
• frontend/src/app/features/book/components/book-browser/book-browser.component.ts

frontend/src/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

frontend/src/app/**/*.{ts,tsx}: Prefer inject() over constructor injection
Follow frontend/eslint.config.js: component selectors use app-*, directive selectors use app*, and any is disallowed

Files:

• frontend/src/app/features/author-browser/components/author-browser/author-browser.component.ts
• frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts
• frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts
• frontend/src/app/shared/components/icon-picker/icon-picker-component.ts
• frontend/src/app/features/book/components/book-browser/book-browser.component.ts

🧠 Learnings (7)

📓 Common learnings

Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to frontend/src/i18n/** : Put user-facing strings in Transloco files under `frontend/src/i18n/`

Learnt from: alexhb1
Repo: grimmory-tools/grimmory PR: 307
File: frontend/src/i18n/hr/layout.json:3-3
Timestamp: 2026-03-31T14:12:39.521Z
Learning: In `frontend/src/i18n/hr/layout.json` (Croatian locale), newly added i18n keys may intentionally use English placeholder values (e.g., `toggleNavigation`, `adjustSidebarWidth`, `closeNavigation`). Do not flag these as missing Croatian translations — this is a deliberate placeholder approach by the team.

📚 Learning: 2026-03-31T14:12:39.521Z

Learnt from: alexhb1
Repo: grimmory-tools/grimmory PR: 307
File: frontend/src/i18n/hr/layout.json:3-3
Timestamp: 2026-03-31T14:12:39.521Z
Learning: In `frontend/src/i18n/hr/layout.json` (Croatian locale), newly added i18n keys may intentionally use English placeholder values (e.g., `toggleNavigation`, `adjustSidebarWidth`, `closeNavigation`). Do not flag these as missing Croatian translations — this is a deliberate placeholder approach by the team.

Applied to files:

• frontend/src/i18n/es/book.json
• frontend/src/app/shared/components/icon-picker/icon-picker-component.html
• frontend/src/i18n/en/shared.json
• frontend/src/i18n/es/shared.json

📚 Learning: 2026-04-04T14:03:28.295Z

Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 372
File: frontend/src/app/features/book/service/app-books-api.service.ts:150-178
Timestamp: 2026-04-04T14:03:28.295Z
Learning: In `frontend/src/app/features/book/service/app-books-api.service.ts`, the `summaryToBook` function intentionally uses `as unknown as Book` to cast a partial object to the `Book` type. The returned object omits some required `Book` fields (e.g. `metadata.bookId`, `pdfProgress.page`) and includes an extra `bookFiles: []` property not in the `Book` interface. This is by design — consumers of `AppBooksApiService.books()` (e.g. `BookCardComponent`) only access the subset of properties that are populated from `AppBookSummary`, so no runtime issues occur. Do not flag this as a type safety error.

Applied to files:

• frontend/src/app/features/book/components/book-browser/book-browser.component.html
• frontend/src/app/features/book/components/book-browser/book-browser.component.ts

📚 Learning: 2026-04-05T21:16:01.715Z

Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 385
File: frontend/src/app/app.component.ts:55-56
Timestamp: 2026-04-05T21:16:01.715Z
Learning: When reviewing code in the Grimmory frontend (Angular), prefer modern Angular patterns. Specifically: (1) Prefer `DestroyRef` with `takeUntilDestroyed(destroyRef)` for teardown in Angular v16+ instead of manually tracking `Subscription` arrays and calling `unsubscribe()` in `ngOnDestroy()`. (2) Prefer `inject()` for dependency injection over constructor injection where appropriate. (3) Prefer Angular signals (e.g., `signal`, `computed`) over `BehaviorSubject`/`Observable` for state where signals/computed values fit the use case. Flag older patterns when they can be replaced with these modern equivalents without changing behavior.

Applied to files:

• frontend/src/app/features/author-browser/components/author-browser/author-browser.component.ts
• frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts
• frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts
• frontend/src/app/shared/components/icon-picker/icon-picker-component.ts
• frontend/src/app/features/book/components/book-browser/book-browser.component.ts

📚 Learning: 2026-04-07T09:28:09.587Z

Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 393
File: frontend/src/app/features/readers/pdf-reader/pdf-reader.component.ts:255-263
Timestamp: 2026-04-07T09:28:09.587Z
Learning: In this Angular frontend (under frontend/src/app/), flag manual resource management/cleanup patterns when there is an Angular v16+ automatic alternative. Examples to prefer: (1) Instead of manually pairing document/window event listeners with stored cleanup functions (e.g., add/removeEventListener with mouseMoveCleanup/documentClickCleanup/keydownCleanup/touchCleanup fields), register teardown via DestroyRef.onDestroy(cleanupFn) (or equivalent Angular v16+ teardown mechanism). (2) Instead of storing Subscriptions in fields and explicitly unsubscribing in ngOnDestroy (e.g., annotationSaveSubscription/annotationCacheSubscription), use takeUntilDestroyed(destroyRef) (piped into the observable) or other Angular v16+ primitives. (3) If teardown is lifecycle-coupled and can be automated via DestroyRef/takeUntilDestroyed/signals (or other Angular v16+ mechanisms), prefer the automated approach over manual ngOnDestroy cleanup. Raise a review finding for the manual pattern and recommend the aut...

Applied to files:

• frontend/src/app/features/author-browser/components/author-browser/author-browser.component.ts
• frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts
• frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts
• frontend/src/app/shared/components/icon-picker/icon-picker-component.ts
• frontend/src/app/features/book/components/book-browser/book-browser.component.ts

📚 Learning: 2026-03-26T01:46:48.863Z

Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to frontend/src/app/**/*.component.ts : Keep Angular code on standalone components. Do not add NgModules

Applied to files:

• frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts
• frontend/src/app/shared/components/icon-picker/icon-picker-component.ts

📚 Learning: 2026-03-26T01:46:48.863Z

Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to frontend/src/i18n/** : Put user-facing strings in Transloco files under `frontend/src/i18n/`

Applied to files:

• frontend/src/app/shared/components/icon-picker/icon-picker-component.html
• frontend/src/i18n/en/shared.json
• frontend/src/app/shared/components/icon-picker/icon-picker-component.ts

🪛 HTMLHint (1.9.2)

frontend/src/app/shared/components/icon-picker/icon-picker-component.html

[error] 1-1: Doctype must be declared before any non-comment content.

(doctype-first)

🔇 Additional comments (11)

frontend/src/i18n/en/book.json (1)

53-59: LGTM!

The new entityTypes translation keys are well-structured and align with the PR objective of replacing hardcoded entity type labels.

frontend/src/app/features/book/components/book-browser/book-browser.component.html (1)

21-21: LGTM!

The entity type label now correctly uses Transloco translations instead of directly interpolating the enum value, addressing the i18n gap.

frontend/src/i18n/es/book.json (1)

53-59: LGTM!

Spanish translations for entityTypes are correct and consistent with the English locale structure.

frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts (1)

275-305: Good use of Intl.DisplayNames for localized language names.

This aligns with the suggestion in Issue #103 to use Intl.DisplayNames instead of a hardcoded array. The fallback chain (Intl → LANGUAGE_NAMES map → capitalize) provides good resilience.

One minor observation: Line 279 calls this.getActiveLang() directly rather than reading from the activeLang signal. Since this method is called from within languageStats (which already tracks the signal), it works correctly, but using this.activeLang() would be more consistent with the signal-based approach.

frontend/src/app/features/book/components/book-browser/book-browser.component.ts (3)

642-653: Language change subscription follows existing patterns in this file.

The implementation correctly updates currentFilterLabel when the language changes. Note that this file uses the older takeUntil(this.destroy$) pattern rather than the modern takeUntilDestroyed(destroyRef), but this is consistent with existing subscriptions in this component (e.g., lines 520-528, 540, 564-569).

---

694-708: LGTM!

The getEntityTypeLabelKey method provides a clean, type-safe mapping from EntityType enum values to i18n keys. The return type literal union ensures only valid keys are returned.

---

458-469: No action needed. All FilterType values have corresponding i18n keys in book.filter.labels for both English and Spanish translation files. The dynamic key construction is correctly supported by the translation files.

frontend/src/app/shared/components/icon-picker/icon-picker-component.html (1)

1-259: Template localization coverage looks solid and reactive.

All newly changed user-facing labels/placeholders/actions are bound through t(...) under the shared.iconPicker prefix, which aligns with the i18n objective and avoids hardcoded English in this component.

As per coding guidelines, "Put user-facing strings in Transloco files under frontend/src/i18n/".

frontend/src/i18n/en/shared.json (1)

154-200: New English iconPicker translation namespace is well-structured and complete.

The added keys cover UI text, validation errors, and toast messages used by the updated icon picker template and runtime logic.

As per coding guidelines, "Put user-facing strings in Transloco files under frontend/src/i18n/".

frontend/src/app/shared/components/icon-picker/icon-picker-component.ts (1)

16-17: Runtime i18n integration in component logic is implemented cleanly.

Using TranslocoService + translateError(...) for validation/loading/save/delete paths removes hardcoded English from logic and keeps messaging centralized.

Also applies to: 43-45, 64-65, 156-157, 181-194, 270-286, 296-297, 308-322, 330-355

frontend/src/i18n/es/shared.json (1)

154-200: Spanish iconPicker translations are in good parity with the new feature keys.

The added namespace covers the same functional areas (UI, errors, toast) needed by the updated icon picker flows.

As per coding guidelines, "Put user-facing strings in Transloco files under frontend/src/i18n/".

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts (1)

159-196: Extract localized option construction into one helper.

The same translation-array construction is duplicated between initial load and lang-change handling. Centralizing it will reduce drift risk.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts`
around lines 159 - 196, The duplicated construction of filterOptions and
sortOptions in ngOnInit and the t.langChanges$ subscription should be extracted
into a single helper method (e.g. buildFilterAndSortOptions or
updateFilterSortOptions) that sets this.filterOptions and this.sortOptions using
this.t.translate; call that helper from ngOnInit and from the subscription
callback (instead of repeating the arrays), and keep the existing
destroyRef/takeUntilDestroyed subscription behavior unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts`:
- Around line 178-196: The language-change subscription in
series-browser.component (this.t.langChanges$) rebuilds filterOptions and
sortOptions but doesn't refresh the browser title; update the subscription to
also call the same title update used at initialization (the same code that calls
this.titleService.setTitle(...) / sets the page title at line ~160) so the page
title is retranslated on language switch—i.e., invoke the component's
title-setting logic (or call the existing method that computes and calls
this.titleService.setTitle) inside the langChanges$ subscribe block.

In
`@frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts`:
- Around line 276-282: The code is force-capitalizing localized language names
which can break locale-specific casing; in the blocks that use Intl.DisplayNames
(the branch guarded by isLanguageCode(lower)), remove the call to capitalize and
return displayName directly (i.e., replace return this.capitalize(displayName)
with return displayName), and make the same change for the other similar block
around the second Intl.DisplayNames usage (the code referencing getActiveLang()
and calling this.capitalize); keep Intl.DisplayNames and the existing guards
(isLanguageCode/getActiveLang) intact.

---

Nitpick comments:
In
`@frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts`:
- Around line 159-196: The duplicated construction of filterOptions and
sortOptions in ngOnInit and the t.langChanges$ subscription should be extracted
into a single helper method (e.g. buildFilterAndSortOptions or
updateFilterSortOptions) that sets this.filterOptions and this.sortOptions using
this.t.translate; call that helper from ngOnInit and from the subscription
callback (instead of repeating the arrays), and keep the existing
destroyRef/takeUntilDestroyed subscription behavior unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 00b2b034-d72f-44b3-b99c-e960636205c5

📥 Commits

Reviewing files that changed from the base of the PR and between 6a9e5b7 and 9a6a933.

📒 Files selected for processing (6)

• frontend/src/app/features/author-browser/components/author-browser/author-browser.component.ts
• frontend/src/app/features/book/components/book-browser/book-browser.component.html
• frontend/src/app/features/book/components/book-browser/book-browser.component.ts
• frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts
• frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts
• frontend/src/app/shared/components/icon-picker/icon-picker-component.ts

🚧 Files skipped from review as they are similar to previous changes (3)

• frontend/src/app/features/author-browser/components/author-browser/author-browser.component.ts
• frontend/src/app/features/book/components/book-browser/book-browser.component.ts
• frontend/src/app/shared/components/icon-picker/icon-picker-component.ts

📜 Review details

⏰ 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). (3)

• GitHub Check: Test Suite / Backend Tests
• GitHub Check: Analyze (javascript-typescript)
• GitHub Check: Analyze (java-kotlin)

🧰 Additional context used

📓 Path-based instructions (3)

frontend/src/**/*.{ts,tsx,html,scss}

📄 CodeRabbit inference engine (AGENTS.md)

Use 2-space indentation in TypeScript, HTML, and SCSS files

Files:

• frontend/src/app/features/book/components/book-browser/book-browser.component.html
• frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts
• frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts

frontend/src/app/**/*.component.ts

📄 CodeRabbit inference engine (AGENTS.md)

Keep Angular code on standalone components. Do not add NgModules

Files:

• frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts
• frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts

frontend/src/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

frontend/src/app/**/*.{ts,tsx}: Prefer inject() over constructor injection
Follow frontend/eslint.config.js: component selectors use app-*, directive selectors use app*, and any is disallowed

Files:

• frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts
• frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts

🧠 Learnings (4)

📓 Common learnings

Learnt from: CR
Repo: grimmory-tools/grimmory PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-26T01:46:48.863Z
Learning: Applies to frontend/src/i18n/** : Put user-facing strings in Transloco files under `frontend/src/i18n/`

Learnt from: alexhb1
Repo: grimmory-tools/grimmory PR: 307
File: frontend/src/i18n/hr/layout.json:3-3
Timestamp: 2026-03-31T14:12:39.521Z
Learning: In `frontend/src/i18n/hr/layout.json` (Croatian locale), newly added i18n keys may intentionally use English placeholder values (e.g., `toggleNavigation`, `adjustSidebarWidth`, `closeNavigation`). Do not flag these as missing Croatian translations — this is a deliberate placeholder approach by the team.

📚 Learning: 2026-04-04T14:03:28.295Z

Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 372
File: frontend/src/app/features/book/service/app-books-api.service.ts:150-178
Timestamp: 2026-04-04T14:03:28.295Z
Learning: In `frontend/src/app/features/book/service/app-books-api.service.ts`, the `summaryToBook` function intentionally uses `as unknown as Book` to cast a partial object to the `Book` type. The returned object omits some required `Book` fields (e.g. `metadata.bookId`, `pdfProgress.page`) and includes an extra `bookFiles: []` property not in the `Book` interface. This is by design — consumers of `AppBooksApiService.books()` (e.g. `BookCardComponent`) only access the subset of properties that are populated from `AppBookSummary`, so no runtime issues occur. Do not flag this as a type safety error.

Applied to files:

• frontend/src/app/features/book/components/book-browser/book-browser.component.html

📚 Learning: 2026-04-05T21:16:01.715Z

Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 385
File: frontend/src/app/app.component.ts:55-56
Timestamp: 2026-04-05T21:16:01.715Z
Learning: When reviewing code in the Grimmory frontend (Angular), prefer modern Angular patterns. Specifically: (1) Prefer `DestroyRef` with `takeUntilDestroyed(destroyRef)` for teardown in Angular v16+ instead of manually tracking `Subscription` arrays and calling `unsubscribe()` in `ngOnDestroy()`. (2) Prefer `inject()` for dependency injection over constructor injection where appropriate. (3) Prefer Angular signals (e.g., `signal`, `computed`) over `BehaviorSubject`/`Observable` for state where signals/computed values fit the use case. Flag older patterns when they can be replaced with these modern equivalents without changing behavior.

Applied to files:

• frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts
• frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts

📚 Learning: 2026-04-07T09:28:09.587Z

Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 393
File: frontend/src/app/features/readers/pdf-reader/pdf-reader.component.ts:255-263
Timestamp: 2026-04-07T09:28:09.587Z
Learning: In this Angular frontend (under frontend/src/app/), flag manual resource management/cleanup patterns when there is an Angular v16+ automatic alternative. Examples to prefer: (1) Instead of manually pairing document/window event listeners with stored cleanup functions (e.g., add/removeEventListener with mouseMoveCleanup/documentClickCleanup/keydownCleanup/touchCleanup fields), register teardown via DestroyRef.onDestroy(cleanupFn) (or equivalent Angular v16+ teardown mechanism). (2) Instead of storing Subscriptions in fields and explicitly unsubscribing in ngOnDestroy (e.g., annotationSaveSubscription/annotationCacheSubscription), use takeUntilDestroyed(destroyRef) (piped into the observable) or other Angular v16+ primitives. (3) If teardown is lifecycle-coupled and can be automated via DestroyRef/takeUntilDestroyed/signals (or other Angular v16+ mechanisms), prefer the automated approach over manual ngOnDestroy cleanup. Raise a review finding for the manual pattern and recommend the aut...

Applied to files:

• frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts
• frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts

🔇 Additional comments (3)

frontend/src/app/features/book/components/book-browser/book-browser.component.html (1)

21-21: Nice fix: entity type is now properly localized and reactive.

Line 21 correctly routes the entity-type label through Transloco (entityTypes.*) instead of rendering raw enum text, which aligns with the PR objective and should update on language switch.

Based on learnings: Put user-facing strings in Transloco files under frontend/src/i18n/.

frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts (1)

5-5: Good reactive teardown pattern here.

Using takeUntilDestroyed(this.destroyRef) in Line 179 is the right lifecycle-safe approach for this subscription.

Based on learnings: Prefer DestroyRef with takeUntilDestroyed(destroyRef) for teardown in Angular v16+ instead of manual subscription cleanup.

Also applies to: 178-180

frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts (1)

142-147: Nice fix for language-change reactivity in chart labels.

The toSignal(...) + computed dependency on activeLang() ensures chart text updates without reload.

Based on learnings: Prefer Angular signals/computed for state where signals fit the use case.

Also applies to: 157-160

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Update the page title on language switches too.

Line 160 sets the title once, but the lang-change handler (Lines 178-196) does not refresh it. The browser title will stay in the old language until navigation/reload.

Suggested fix

     this.t.langChanges$
       .pipe(takeUntilDestroyed(this.destroyRef))
       .subscribe(() => {
+        this.pageTitle.setPageTitle(this.t.translate('seriesBrowser.pageTitle'));
         this.filterOptions = [
           {label: this.t.translate('seriesBrowser.filters.all'), value: 'all'},
           {label: this.t.translate('seriesBrowser.filters.notStarted'), value: 'not-started'},

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts`
around lines 178 - 196, The language-change subscription in
series-browser.component (this.t.langChanges$) rebuilds filterOptions and
sortOptions but doesn't refresh the browser title; update the subscription to
also call the same title update used at initialization (the same code that calls
this.titleService.setTitle(...) / sets the page title at line ~160) so the page
title is retranslated on language switch—i.e., invoke the component's
title-setting logic (or call the existing method that computes and calls
this.titleService.setTitle) inside the langChanges$ subscribe block.

[zachyale]

Honestly if there are this many places we need to be dynamic to update localization we likely need a more holistic approach that is transferable to other components

[coderabbitai]

@zachyale — that's a very valid architectural observation. Transloco actually ships a first-class solution for this: translateSignal() from @jsverse/transloco, which is integrated with Angular's dependency injection system and can be called within an injection context. It returns a signal that automatically re-evaluates when the active language changes — no manual subscription or ngOnInit wiring needed.

For option arrays like filterOptions/sortOptions, the idiomatic holistic approach is to:

1. Create a single activeLang signal by bridging langChanges$ via toSignal() (your language-chart.component.ts in this same PR already demonstrates this pattern).
2. Express the option arrays as computed() signals that read activeLang() — they will automatically recompute on language change.

Here is how series-browser.component.ts would look with this approach:

// Bridge langChanges$ once — no subscription, no ngOnDestroy wiring needed
private readonly activeLang = toSignal(this.t.langChanges$, {
  initialValue: this.t.getActiveLang()
});

// Reactive option arrays — recompute automatically when activeLang() changes
readonly filterOptions = computed<FilterOption[]>(() => {
  this.activeLang(); // establish reactive dependency
  return [
    { label: this.t.translate('seriesBrowser.filters.all'),        value: 'all' },
    { label: this.t.translate('seriesBrowser.filters.notStarted'), value: 'not-started' },
    { label: this.t.translate('seriesBrowser.filters.inProgress'), value: 'in-progress' },
    { label: this.t.translate('seriesBrowser.filters.completed'),  value: 'completed' },
    { label: this.t.translate('seriesBrowser.filters.abandoned'),  value: 'abandoned' },
  ];
});

readonly sortOptions = computed<SortOption[]>(() => {
  this.activeLang();
  return [
    { label: this.t.translate('seriesBrowser.sort.nameAsc'),      value: 'name-asc' },
    { label: this.t.translate('seriesBrowser.sort.nameDesc'),     value: 'name-desc' },
    { label: this.t.translate('seriesBrowser.sort.bookCount'),    value: 'book-count' },
    { label: this.t.translate('seriesBrowser.sort.progress'),     value: 'progress' },
    { label: this.t.translate('seriesBrowser.sort.recentlyRead'), value: 'recently-read' },
    { label: this.t.translate('seriesBrowser.sort.recentlyAdded'), value: 'recently-added' },
  ];
});

For single translated strings (like the page title), translateSignal is even cleaner:

// Automatically reactive — updates with language changes
private readonly pageTitleLabel = translateSignal('seriesBrowser.pageTitle');

// Then in ngOnInit, or an effect():
effect(() => this.pageTitle.setPageTitle(this.pageTitleLabel()));

The same activeLang + computed() pattern can be extracted into a shared utility or base helper so every component that needs reactive option arrays only calls toSignal(t.langChanges$) once and uses computed() for everything derived from it — no repeated subscribe/ngOnDestroy boilerplate across the codebase. This would apply uniformly to author-browser, series-browser, book-browser, and any future components.

Would you like me to plan these changes as a refactor across all affected components?

---

✏️ Learnings added

Learnt from: zachyale
Repo: grimmory-tools/grimmory PR: 439
File: frontend/src/app/features/series-browser/components/series-browser/series-browser.component.ts:178-196
Timestamp: 2026-04-11T03:55:57.229Z
Learning: In the Grimmory Angular frontend (frontend/src/app/), the preferred holistic approach for reactive localization (auto-updating on language switch) is: (1) Use `translateSignal()` from `jsverse/transloco` for individual reactive translated strings. (2) For option/label arrays, create a single `activeLang` signal via `toSignal(t.langChanges$, { initialValue: t.getActiveLang() })` and derive option arrays as `computed()` signals that read `activeLang()` — this eliminates all manual `langChanges$` subscriptions and `ngOnDestroy` wiring. The team explicitly prefers this signal/computed pattern over manual subscribe-in-ngOnInit approaches when implementing reactive i18n.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

🧠 Learnings used

Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 385
File: frontend/src/app/app.component.ts:55-56
Timestamp: 2026-04-05T21:16:01.715Z
Learning: When reviewing code in the Grimmory frontend (Angular), prefer modern Angular patterns. Specifically: (1) Prefer `DestroyRef` with `takeUntilDestroyed(destroyRef)` for teardown in Angular v16+ instead of manually tracking `Subscription` arrays and calling `unsubscribe()` in `ngOnDestroy()`. (2) Prefer `inject()` for dependency injection over constructor injection where appropriate. (3) Prefer Angular signals (e.g., `signal`, `computed`) over `BehaviorSubject`/`Observable` for state where signals/computed values fit the use case. Flag older patterns when they can be replaced with these modern equivalents without changing behavior.

---

Learnt from: balazs-szucs
Repo: grimmory-tools/grimmory PR: 393
File: frontend/src/app/features/readers/pdf-reader/pdf-reader.component.ts:255-263
Timestamp: 2026-04-07T09:28:09.587Z
Learning: In this Angular frontend (under frontend/src/app/), flag manual resource management/cleanup patterns when there is an Angular v16+ automatic alternative. Examples to prefer: (1) Instead of manually pairing document/window event listeners with stored cleanup functions (e.g., add/removeEventListener with mouseMoveCleanup/documentClickCleanup/keydownCleanup/touchCleanup fields), register teardown via DestroyRef.onDestroy(cleanupFn) (or equivalent Angular v16+ teardown mechanism). (2) Instead of storing Subscriptions in fields and explicitly unsubscribing in ngOnDestroy (e.g., annotationSaveSubscription/annotationCacheSubscription), use takeUntilDestroyed(destroyRef) (piped into the observable) or other Angular v16+ primitives. (3) If teardown is lifecycle-coupled and can be automated via DestroyRef/takeUntilDestroyed/signals (or other Angular v16+ mechanisms), prefer the automated approach over manual ngOnDestroy cleanup. Raise a review finding for the manual pattern and recommend the automated alternative.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Avoid forcing capitalization on localized language names.

Intl.DisplayNames already returns locale-correct labels. Capitalizing them (Line 281) can produce linguistically incorrect casing for some locales.

Suggested fix

     if (this.isLanguageCode(lower)) {
       try {
         const displayNames = new Intl.DisplayNames([this.getActiveLang()], {type: 'language'});
         const displayName = displayNames.of(lower);
         if (displayName) {
-          return this.capitalize(displayName);
+          return displayName;
         }
       } catch {
         // Fall through to existing mapping behavior.
       }
     }

Also applies to: 291-300

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@frontend/src/app/features/stats/component/library-stats/charts/language-chart/language-chart.component.ts`
around lines 276 - 282, The code is force-capitalizing localized language names
which can break locale-specific casing; in the blocks that use Intl.DisplayNames
(the branch guarded by isLanguageCode(lower)), remove the call to capitalize and
return displayName directly (i.e., replace return this.capitalize(displayName)
with return displayName), and make the same change for the other similar block
around the second Intl.DisplayNames usage (the code referencing getActiveLang()
and calling this.capitalize); keep Intl.DisplayNames and the existing guards
(isLanguageCode/getActiveLang) intact.