refactor(ui): replace SVG artwork placeholders with CSS #292
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📜 Recent 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). (4)
🧰 Additional context used📓 Path-based instructions (2)frontend/src/**/*.{ts,tsx,html,scss}📄 CodeRabbit inference engine (AGENTS.md)
Files:
frontend/src/app/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (4)
📝 WalkthroughWalkthroughReplaces the old SVG-based cover generator with a reusable CoverPlaceholderComponent, makes URL helper methods return nullable URLs (no autogenerated fallbacks), and updates many templates/types to conditionally render placeholders when cover URLs are absent. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
🤣 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
frontend/src/app/features/readers/ebook-reader/layout/sidebar/sidebar.component.html (1)
17-18: Make placeholder cover interaction consistent with image coverWhen
coverUrlis missing, the fallback cover is no longer clickable, while the image cover opens metadata. Consider giving the placeholder the same click/keyboard behavior for parity.Proposed change
- } `@else` { - <app-cover-placeholder class="book-cover" [title]="bookInfo().title" [author]="bookInfo().authors" size="sm" /> + } `@else` { + <button + type="button" + class="book-cover" + (click)="onCoverClick()" + (keydown.enter)="onCoverClick()" + aria-label="Open book metadata"> + <app-cover-placeholder [title]="bookInfo().title" [author]="bookInfo().authors" size="sm" /> + </button> }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/features/readers/ebook-reader/layout/sidebar/sidebar.component.html` around lines 17 - 18, The placeholder app-cover-placeholder should have the same click and keyboard behavior as the image cover: add the same click handler used by the image cover (e.g., (click)="openMetadata(bookInfo())") and make it keyboard-accessible by adding tabindex="0" and keydown handlers for Enter/Space that call the same openMetadata method; also add role="button" and an appropriate aria-label (e.g., aria-label="Open book metadata for {{bookInfo().title}}") so the fallback cover (when coverUrl is missing) behaves identically to the image cover.
🤖 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/book/components/book-browser/book-card/book-card.component.html`:
- Around line 17-19: The fallback branch for missing covers uses
<app-cover-placeholder> but omits the same CSS classes used by the image branch,
breaking sizing/aspect styles; update the <app-cover-placeholder> element in
book-card.component.html to include the same class names and state bindings as
the image branch (e.g., add the "book-cover" class and the "square-cover" state
class/binding used there) so the placeholder and image render with identical
layout/styling behavior.
In
`@frontend/src/app/features/book/components/book-card-lite/book-card-lite-component.html`:
- Around line 8-10: The placeholder branch in book-card-lite-component.html is
missing the book-cover CSS class used by the real image branch; update the
<app-cover-placeholder> element (used in the else branch of the card rendering)
to include the same "book-cover" class so its styling and layout match the <img>
branch (keep existing inputs title and author intact).
In
`@frontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-viewer.component.html`:
- Around line 9-12: The cover image currently uses a generic alt="Image"; update
the <p-image> in metadata-viewer.component.html to use a meaningful alt or an
empty decorative alt by binding alt to a component property (e.g.,
[alt]="coverAlt") instead of the literal "Image"; set coverAlt in the
MetadataViewerComponent to a book-specific string (e.g., `${book.title} cover`
or `${metadata.title} cover`) when a title exists, and set it to an empty string
("") when the image is purely decorative or the title is absent.
In
`@frontend/src/app/features/notebook/components/notebook/notebook.component.ts`:
- Line 21: The ebook grouping path still calls getDirectThumbnailUrl(...) which
always returns a string, preventing the nullable thumbnailUrl contract from
taking effect; change the ebook branch to use the nullable thumbnail source
(thumbnailUrl) instead of getDirectThumbnailUrl and ensure any cover-update
metadata is forwarded through that same nullable property so the placeholder
fallback can trigger for ebooks without artwork. Locate the ebook grouping logic
and replace the direct call to getDirectThumbnailUrl with the nullable
thumbnailUrl value (or a nullable wrapper function) and propagate cover-update
metadata alongside thumbnailUrl when building the ebook entry.
---
Nitpick comments:
In
`@frontend/src/app/features/readers/ebook-reader/layout/sidebar/sidebar.component.html`:
- Around line 17-18: The placeholder app-cover-placeholder should have the same
click and keyboard behavior as the image cover: add the same click handler used
by the image cover (e.g., (click)="openMetadata(bookInfo())") and make it
keyboard-accessible by adding tabindex="0" and keydown handlers for Enter/Space
that call the same openMetadata method; also add role="button" and an
appropriate aria-label (e.g., aria-label="Open book metadata for
{{bookInfo().title}}") so the fallback cover (when coverUrl is missing) behaves
identically to the image cover.
🪄 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: 3f17033a-42f6-4375-ba45-ce0f24c1703f
📒 Files selected for processing (34)
frontend/src/app/features/book/components/book-browser/book-card/book-card.component.htmlfrontend/src/app/features/book/components/book-browser/book-card/book-card.component.tsfrontend/src/app/features/book/components/book-browser/book-table/book-table.component.htmlfrontend/src/app/features/book/components/book-browser/book-table/book-table.component.tsfrontend/src/app/features/book/components/book-card-lite/book-card-lite-component.htmlfrontend/src/app/features/book/components/book-card-lite/book-card-lite-component.tsfrontend/src/app/features/book/components/book-searcher/book-searcher.component.htmlfrontend/src/app/features/book/components/book-searcher/book-searcher.component.tsfrontend/src/app/features/book/components/duplicate-merger/duplicate-merger.component.htmlfrontend/src/app/features/book/components/duplicate-merger/duplicate-merger.component.tsfrontend/src/app/features/book/components/series-page/series-page.component.htmlfrontend/src/app/features/book/components/series-page/series-page.component.tsfrontend/src/app/features/metadata/component/book-metadata-center/metadata-editor/metadata-editor.component.htmlfrontend/src/app/features/metadata/component/book-metadata-center/metadata-editor/metadata-editor.component.tsfrontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-tabs/metadata-tabs.component.htmlfrontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-tabs/metadata-tabs.component.tsfrontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-viewer.component.htmlfrontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-viewer.component.tsfrontend/src/app/features/notebook/components/notebook/notebook.component.htmlfrontend/src/app/features/notebook/components/notebook/notebook.component.tsfrontend/src/app/features/readers/cbx-reader/layout/sidebar/cbx-sidebar.component.htmlfrontend/src/app/features/readers/cbx-reader/layout/sidebar/cbx-sidebar.component.tsfrontend/src/app/features/readers/ebook-reader/dialogs/metadata-dialog.component.htmlfrontend/src/app/features/readers/ebook-reader/dialogs/metadata-dialog.component.scssfrontend/src/app/features/readers/ebook-reader/dialogs/metadata-dialog.component.tsfrontend/src/app/features/readers/ebook-reader/layout/sidebar/sidebar.component.htmlfrontend/src/app/features/readers/ebook-reader/layout/sidebar/sidebar.component.tsfrontend/src/app/features/series-browser/components/series-card/series-card.component.htmlfrontend/src/app/features/series-browser/components/series-card/series-card.component.spec.tsfrontend/src/app/features/series-browser/components/series-card/series-card.component.tsfrontend/src/app/shared/components/cover-generator/cover-generator.component.spec.tsfrontend/src/app/shared/components/cover-generator/cover-generator.component.tsfrontend/src/app/shared/service/url-helper.service.spec.tsfrontend/src/app/shared/service/url-helper.service.ts
💤 Files with no reviewable changes (1)
- frontend/src/app/features/readers/ebook-reader/dialogs/metadata-dialog.component.scss
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
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/duplicate-merger/duplicate-merger.component.tsfrontend/src/app/features/notebook/components/notebook/notebook.component.tsfrontend/src/app/features/book/components/book-browser/book-table/book-table.component.tsfrontend/src/app/features/book/components/book-searcher/book-searcher.component.tsfrontend/src/app/features/notebook/components/notebook/notebook.component.htmlfrontend/src/app/features/readers/ebook-reader/layout/sidebar/sidebar.component.tsfrontend/src/app/features/series-browser/components/series-card/series-card.component.spec.tsfrontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-tabs/metadata-tabs.component.tsfrontend/src/app/features/readers/ebook-reader/layout/sidebar/sidebar.component.htmlfrontend/src/app/features/readers/ebook-reader/dialogs/metadata-dialog.component.htmlfrontend/src/app/features/readers/cbx-reader/layout/sidebar/cbx-sidebar.component.htmlfrontend/src/app/features/readers/cbx-reader/layout/sidebar/cbx-sidebar.component.tsfrontend/src/app/features/book/components/book-searcher/book-searcher.component.htmlfrontend/src/app/features/book/components/book-card-lite/book-card-lite-component.tsfrontend/src/app/features/metadata/component/book-metadata-center/metadata-editor/metadata-editor.component.tsfrontend/src/app/features/series-browser/components/series-card/series-card.component.htmlfrontend/src/app/features/book/components/book-browser/book-card/book-card.component.htmlfrontend/src/app/features/readers/ebook-reader/dialogs/metadata-dialog.component.tsfrontend/src/app/features/book/components/book-browser/book-table/book-table.component.htmlfrontend/src/app/features/book/components/book-card-lite/book-card-lite-component.htmlfrontend/src/app/features/series-browser/components/series-card/series-card.component.tsfrontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-tabs/metadata-tabs.component.htmlfrontend/src/app/features/book/components/series-page/series-page.component.tsfrontend/src/app/features/book/components/book-browser/book-card/book-card.component.tsfrontend/src/app/features/book/components/series-page/series-page.component.htmlfrontend/src/app/shared/service/url-helper.service.spec.tsfrontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-viewer.component.tsfrontend/src/app/shared/components/cover-generator/cover-generator.component.spec.tsfrontend/src/app/features/book/components/duplicate-merger/duplicate-merger.component.htmlfrontend/src/app/shared/components/cover-generator/cover-generator.component.tsfrontend/src/app/shared/service/url-helper.service.tsfrontend/src/app/features/metadata/component/book-metadata-center/metadata-editor/metadata-editor.component.htmlfrontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-viewer.component.html
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/book/components/duplicate-merger/duplicate-merger.component.tsfrontend/src/app/features/notebook/components/notebook/notebook.component.tsfrontend/src/app/features/book/components/book-browser/book-table/book-table.component.tsfrontend/src/app/features/book/components/book-searcher/book-searcher.component.tsfrontend/src/app/features/readers/ebook-reader/layout/sidebar/sidebar.component.tsfrontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-tabs/metadata-tabs.component.tsfrontend/src/app/features/readers/cbx-reader/layout/sidebar/cbx-sidebar.component.tsfrontend/src/app/features/metadata/component/book-metadata-center/metadata-editor/metadata-editor.component.tsfrontend/src/app/features/readers/ebook-reader/dialogs/metadata-dialog.component.tsfrontend/src/app/features/series-browser/components/series-card/series-card.component.tsfrontend/src/app/features/book/components/series-page/series-page.component.tsfrontend/src/app/features/book/components/book-browser/book-card/book-card.component.tsfrontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-viewer.component.tsfrontend/src/app/shared/components/cover-generator/cover-generator.component.ts
frontend/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
frontend/src/app/**/*.{ts,tsx}: Preferinject()over constructor injection
Followfrontend/eslint.config.js: component selectors useapp-*, directive selectors useapp*, andanyis disallowed
Files:
frontend/src/app/features/book/components/duplicate-merger/duplicate-merger.component.tsfrontend/src/app/features/notebook/components/notebook/notebook.component.tsfrontend/src/app/features/book/components/book-browser/book-table/book-table.component.tsfrontend/src/app/features/book/components/book-searcher/book-searcher.component.tsfrontend/src/app/features/readers/ebook-reader/layout/sidebar/sidebar.component.tsfrontend/src/app/features/series-browser/components/series-card/series-card.component.spec.tsfrontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-tabs/metadata-tabs.component.tsfrontend/src/app/features/readers/cbx-reader/layout/sidebar/cbx-sidebar.component.tsfrontend/src/app/features/book/components/book-card-lite/book-card-lite-component.tsfrontend/src/app/features/metadata/component/book-metadata-center/metadata-editor/metadata-editor.component.tsfrontend/src/app/features/readers/ebook-reader/dialogs/metadata-dialog.component.tsfrontend/src/app/features/series-browser/components/series-card/series-card.component.tsfrontend/src/app/features/book/components/series-page/series-page.component.tsfrontend/src/app/features/book/components/book-browser/book-card/book-card.component.tsfrontend/src/app/shared/service/url-helper.service.spec.tsfrontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-viewer.component.tsfrontend/src/app/shared/components/cover-generator/cover-generator.component.spec.tsfrontend/src/app/shared/components/cover-generator/cover-generator.component.tsfrontend/src/app/shared/service/url-helper.service.ts
frontend/src/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest for tests in the frontend
Files:
frontend/src/app/features/series-browser/components/series-card/series-card.component.spec.tsfrontend/src/app/shared/service/url-helper.service.spec.tsfrontend/src/app/shared/components/cover-generator/cover-generator.component.spec.ts
🧠 Learnings (2)
📚 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/book/components/duplicate-merger/duplicate-merger.component.tsfrontend/src/app/features/notebook/components/notebook/notebook.component.tsfrontend/src/app/features/book/components/book-browser/book-table/book-table.component.tsfrontend/src/app/features/book/components/book-searcher/book-searcher.component.tsfrontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-tabs/metadata-tabs.component.tsfrontend/src/app/features/book/components/book-card-lite/book-card-lite-component.tsfrontend/src/app/features/book/components/book-browser/book-card/book-card.component.tsfrontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-viewer.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/**/*.{test,spec}.ts : Use Vitest for tests in the frontend
Applied to files:
frontend/src/app/shared/components/cover-generator/cover-generator.component.spec.ts
🔇 Additional comments (37)
frontend/src/app/features/book/components/book-searcher/book-searcher.component.ts (1)
12-12: Placeholder component wiring is correct.The
CoverPlaceholderComponentimport and standaloneimportsregistration are clean and match the fallback rendering intent.Also applies to: 31-31
frontend/src/app/features/book/components/duplicate-merger/duplicate-merger.component.ts (1)
18-18: Standalone import update looks good.
CoverPlaceholderComponentis correctly added for template fallback support.Also applies to: 43-43
frontend/src/app/features/notebook/components/notebook/notebook.component.ts (1)
15-15: Import wiring for placeholder rendering is correct.The
CoverPlaceholderComponentregistration itself is correct and aligns with the template fallback.Also applies to: 47-47
frontend/src/app/features/book/components/book-browser/book-table/book-table.component.ts (1)
10-10: Book table placeholder integration looks correct.The added import and standalone
importsupdate are consistent with the null-cover fallback pattern.Also applies to: 33-34
frontend/src/app/features/series-browser/components/series-card/series-card.component.spec.ts (1)
96-96: Spec update matches the new nullable API contract.
toBeNull()here is the right assertion for missing cover indices.frontend/src/app/features/readers/cbx-reader/layout/sidebar/cbx-sidebar.component.ts (1)
8-8: Sidebar placeholder dependency is correctly registered.No concerns with this standalone import change.
Also applies to: 15-15
frontend/src/app/features/metadata/component/book-metadata-center/metadata-editor/metadata-editor.component.ts (1)
10-10: Metadata editor placeholder wiring is good.The new component import and registration are correct for fallback rendering in the template.
Also applies to: 61-61
frontend/src/app/features/notebook/components/notebook/notebook.component.html (1)
141-145: Template fallback branch is implemented correctly.The
@if/@elserendering between<img>and<app-cover-placeholder>is clean and readable.frontend/src/app/features/readers/ebook-reader/layout/sidebar/sidebar.component.ts (1)
8-8: Standalone imports update looks correct
CoverPlaceholderComponentis correctly imported and registered inimports, matching the template fallback usage.Also applies to: 15-15
frontend/src/app/features/readers/cbx-reader/layout/sidebar/cbx-sidebar.component.html (1)
11-12: Fallback rendering is clean and consistentGood use of
<app-cover-placeholder>in the no-cover branch with title/author bindings.frontend/src/app/features/readers/ebook-reader/dialogs/metadata-dialog.component.html (1)
15-15: Shared placeholder usage is a good simplificationReplacing the inline fallback with the shared component here is clean and consistent.
frontend/src/app/features/book/components/duplicate-merger/duplicate-merger.component.html (1)
189-196: Cover URL guard and fallback are well implementedThe
as coverUrlguard with a placeholder fallback avoids invalid image sources and keeps the row rendering robust.frontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-tabs/metadata-tabs.component.ts (1)
14-14: Standalone component wiring is correct
CoverPlaceholderComponentis added cleanly to imports and aligns with template usage.Based on learnings: "Applies to frontend/src/app/**/*.component.ts : Keep Angular code on standalone components. Do not add NgModules".
Also applies to: 77-78
frontend/src/app/features/series-browser/components/series-card/series-card.component.html (1)
4-19: Stacked cover fallback is implemented correctlyGreat branch parity here—placeholder keeps the same class/positioning hooks as the image variant.
frontend/src/app/features/book/components/book-card-lite/book-card-lite-component.ts (1)
45-50: Nullable thumbnail flow looks correct.The
string | nullreturn type and URL selection logic are consistent with the placeholder-based rendering path.frontend/src/app/features/book/components/book-browser/book-table/book-table.component.html (1)
54-77: Cover fallback handling is consistent here.Good update: both thumbnail and tooltip now gracefully render a placeholder when the URL is unavailable.
frontend/src/app/features/book/components/book-searcher/book-searcher.component.html (1)
52-60: Nice null-cover fallback in search results.This avoids broken image states and keeps rendering predictable for missing artwork.
frontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-viewer.component.ts (1)
1285-1290:getBookCoverUrlnullability update is solid.Returning
string | nullhere cleanly supports the new template-level placeholder branch.frontend/src/app/features/readers/ebook-reader/dialogs/metadata-dialog.component.ts (1)
3-12: Imports update looks good.Standalone imports now include the placeholder component and remain consistent with the dialog template behavior.
frontend/src/app/features/book/components/series-page/series-page.component.ts (1)
484-489: Series thumbnail nullability change is clean.
getBookThumbnailUrlnow correctly propagates missing-cover states for template-level placeholder rendering.frontend/src/app/features/metadata/component/book-metadata-center/metadata-viewer/metadata-tabs/metadata-tabs.component.html (1)
76-103: LGTM!The conditional rendering pattern using
@if (...; as coverUrl)correctly handles the nullable return type from the URL helper methods. The fallback to<app-cover-placeholder>with appropriate title, author, and size inputs is well-implemented.frontend/src/app/features/series-browser/components/series-card/series-card.component.ts (1)
38-45: LGTM!The return type change to
string | nulland returningnullfor missing cover books aligns correctly with the updatedUrlHelperServicecontract. The template can now use@ifto conditionally render the placeholder.frontend/src/app/features/metadata/component/book-metadata-center/metadata-editor/metadata-editor.component.html (2)
20-31: LGTM!The ebook cover section correctly uses conditional rendering with the nullable
getCoverUrl()return value, falling back to the CSS placeholder with appropriate dimensions and inputs.
101-113: LGTM!The audiobook cover section mirrors the ebook cover pattern with correct square dimensions (200x200px) for the placeholder fallback.
frontend/src/app/features/book/components/book-browser/book-card/book-card.component.ts (2)
86-86: LGTM!The field type change from
stringinitialized to''tostring | nullinitialized tonullcorrectly reflects the updated nullable contract fromUrlHelperService.
218-220: LGTM!The getter return type update to
string | nullmaintains type safety and enables proper conditional rendering in the template.frontend/src/app/features/book/components/series-page/series-page.component.html (3)
35-44: LGTM!The hero cover section correctly conditionally renders the thumbnail image when available, falling back to the CSS placeholder with
size="lg"for the larger hero display.
193-200: LGTM!The "next up" thumbnail correctly uses conditional rendering and falls back to the smaller placeholder (
size="sm"), appropriately sized for the compact card layout.
321-328: LGTM!The book list thumbnail follows the same pattern with
size="sm"for the compact list view, maintaining consistency across the component.frontend/src/app/shared/service/url-helper.service.spec.ts (2)
50-70: LGTM!The new tests correctly verify the nullable return behavior when
coverUpdatedOnoraudiobookCoverUpdatedOnparameters are not provided. Each URL helper method has appropriate null-case coverage.
76-81: LGTM!Good coverage for the auth-unavailable edge case, verifying that direct URLs are returned without a token when authentication is not present.
frontend/src/app/shared/components/cover-generator/cover-generator.component.spec.ts (2)
21-47: LGTM!Comprehensive DOM-focused tests for
CoverPlaceholderComponentcovering text rendering, background color application, and size class assignment. The use offixture.componentRef.setInput()is the correct approach for testing Angular signal-based inputs.
50-66: LGTM!Good unit test coverage for the
coverColorForutility function, verifying determinism, differentiation for different inputs, and valid hex color format output.frontend/src/app/shared/components/cover-generator/cover-generator.component.ts (2)
3-19: LGTM!The color generation system is well-designed: a curated palette of dark colors combined with a deterministic hash function ensures consistent placeholder colors for the same book across the application while providing visual variety.
23-105: LGTM!The
CoverPlaceholderComponentis a clean, self-contained standalone component using Angular's modern signal-based inputs and computed properties. The inline template with CSS line-clamping provides a simple but effective placeholder that aligns with the PR's goal of replacing the complex SVG generation pipeline.frontend/src/app/shared/service/url-helper.service.ts (2)
25-27: LGTM - Nullable return enables placeholder fallback.The return type change to
string | nullcorrectly signals when no cover exists, allowing templates to conditionally render the CSS placeholder. The query string pattern (using timestamp directly) is consistent withgetDirectThumbnailUrl.
38-55: Consistent nullable pattern across all cover/thumbnail methods.All four URL methods (
getCoverUrl,getAudiobookCoverUrl,getAudiobookThumbnailUrl, and the earliergetThumbnailUrl) now follow the same contract: returnnullwhen no cover timestamp exists, enabling the UI to render CSS placeholders instead of making requests for non-existent images. The context snippets confirm callers properly guard against null values.
|
@alexhb1 i had a quick look at the rabbit comments and they all look reasonable - mind taking a pass? |
|
Just working through them now :) |
|
this one is good to go |
…ls#292) * chore(ui): replace SVG placeholders with CSS alternative * fix(ui): Placeholder review followup
…ls#292) * chore(ui): replace SVG placeholders with CSS alternative * fix(ui): Placeholder review followup
* chore(ui): replace SVG placeholders with CSS alternative * fix(ui): Placeholder review followup
* chore(ui): replace SVG placeholders with CSS alternative * fix(ui): Placeholder review followup
…ls#292) * chore(ui): replace SVG placeholders with CSS alternative * fix(ui): Placeholder review followup
Description
Currently, books with no artwork use a generated SVG with specific (ugly) styling and has a complex and intensive pipeline for displaying them, especially with large libraries. They're generated on the fly with no storage or caching. This replaces the entire approach with a simplified in-line placeholder div with a bit of basic text and CSS.
Before vs after:
Changes
Will mark as ready once my other layout shell rework PR is done and through, this can be sorted afterwards.
Summary by CodeRabbit
New Features
Bug Fixes
Tests