perf(ui): captures some low hanging fruit around excessive re-renders in the app by ZYancey · Pull Request #158 · grimmory-tools/grimmory

ZYancey · 2026-03-24T01:17:31Z

Description

This cleans up some issues I noted in discord around there being a few small performance bugs I noticed when skimming the repo, I'll push the others up over the course of the week but they should all be pretty small like this PR

(AI Generated Description as follows)
Adds ChangeDetectionStrategy.OnPush to 12 presentational components identified via Angular DevTools profiler as high-frequency change detection targets. All components were verified to have no unmanaged subscriptions, no internal BehaviorSubject/Subject state, and no mutations outside of @Input() or template-event paths — making them safe for OnPush without markForCheck() changes.

Profiled on the current develop tip (post-TanStack migration) before and after to isolate the contribution of this change specifically.
Profiler results (develop → this branch)

Metric	Before	After	Delta
CD cycles	395	313	-21%
Total CD time	2,397ms	1,750ms	-27%
rAF-triggered cycles	252 (1,165ms)	166 (734ms)	-37%
BookCardComponent CD cost	250ms (4,480 checks)	27ms (6,025 checks)	-89%
SeriesCardComponent CD cost	100ms	0ms	-100%
ThemeConfiguratorComponent CD cost	45ms	0ms	-100%
Tooltip lifecycle hooks	237ms	79ms	-67%
TieredMenu lifecycle hooks	103ms	17ms	-83%

BookCardComponent's check count increasing while its cost drops is expected OnPush behavior — Angular visits the node to decide whether to check it, then skips the subtree when inputs are unchanged.

Linked Issue: Fixes #157

Changes

Component	Reason safe for OnPush
SeriesCardComponent	Pure @ Input/@ Output, no subscriptions
SeriesScrollerComponent	Pure @ Input/@ Output, no subscriptions
TagComponent	All Signal inputs + computed()
ThemeConfiguratorComponent	All Signal-based (computed, effect)
BookCardLiteComponent	Signal-based after #117; isHovered set via template events
DashboardScrollerComponent	@ Input-driven; openMenuBookId only mutated by template events
MultiSortPopoverComponent	All mutations via template events, emits via @ Output
ReaderIconComponent	Pure @ Input, static SVG renderer
SettingsApplicationModeComponent	@ Input-driven; scope values set by template events
CoverGeneratorComponent	Pure @ Input, empty template
EmptyComponent	No inputs, no logic
ExternalDocLinkComponent	Pure @ Input, click opens URL

What was intentionally excluded
BookBrowserComponent — internal BehaviorSubject state and ChangeDetectorRef injection; safe migration requires verifying all mutation paths first
AppMenuitemComponent — 4 unmanaged subscriptions; needs cleanup before OnPush
SeriesBrowserComponent — @ HostListener('window:resize') mutates template-bound computed properties
Reader components — deferred per contribution guidelines

Summary by CodeRabbit

Refactor
- Optimized change detection behavior across multiple UI components for improved performance and responsiveness throughout the application.

… in the app

coderabbitai · 2026-03-24T01:17:47Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7b3ba752-3a8f-4125-8df5-f6d4fbbf7d1f

📥 Commits

Reviewing files that changed from the base of the PR and between 3c7550c and 54a3846.

📒 Files selected for processing (12)

booklore-ui/src/app/features/book/components/book-browser/sorting/multi-sort-popover/multi-sort-popover.component.ts
booklore-ui/src/app/features/book/components/book-card-lite/book-card-lite-component.ts
booklore-ui/src/app/features/dashboard/components/dashboard-scroller/dashboard-scroller.component.ts
booklore-ui/src/app/features/readers/ebook-reader/shared/icon.component.ts
booklore-ui/src/app/features/series-browser/components/series-card/series-card.component.ts
booklore-ui/src/app/features/series-browser/components/series-scroller/series-scroller.component.ts
booklore-ui/src/app/features/settings/reader-preferences/settings-application-mode/settings-application-mode.component.ts
booklore-ui/src/app/shared/components/cover-generator/cover-generator.component.ts
booklore-ui/src/app/shared/components/empty/empty.component.ts
booklore-ui/src/app/shared/components/external-doc-link/external-doc-link.component.ts
booklore-ui/src/app/shared/components/tag/tag.component.ts
booklore-ui/src/app/shared/layout/component/theme-configurator/theme-configurator.component.ts

📝 Walkthrough

Walkthrough

Added ChangeDetectionStrategy.OnPush to 12 presentational components across book, dashboard, reader, series, settings, and shared features. Each component now optimizes change detection by only checking when @Input() properties change, reducing unnecessary tree traversals.

Changes

Cohort / File(s)	Summary
Book Features `src/app/features/book/components/.../multi-sort-popover.component.ts`, `src/app/features/book/components/book-card-lite-component.ts`	Added `ChangeDetectionStrategy.OnPush` to improve change detection efficiency for book browser and card components.
Dashboard Features `src/app/features/dashboard/components/dashboard-scroller/dashboard-scroller.component.ts`	Enabled OnPush change detection for the scroller component.
Reader Features `src/app/features/readers/ebook-reader/shared/icon.component.ts`	Applied OnPush strategy to icon rendering component.
Series Features `src/app/features/series-browser/components/.../series-card.component.ts`, `src/app/features/series-browser/components/.../series-scroller.component.ts`	Updated series browser card and scroller components with OnPush change detection.
Settings Features `src/app/features/settings/reader-preferences/.../settings-application-mode.component.ts`	Enabled OnPush for application mode settings component.
Shared Presentation Components `src/app/shared/components/cover-generator/cover-generator.component.ts`, `src/app/shared/components/empty/empty.component.ts`, `src/app/shared/components/external-doc-link/external-doc-link.component.ts`, `src/app/shared/components/tag/tag.component.ts`	Applied OnPush strategy to four shared presentational components that receive data exclusively via `@Input()`.
Shared Layout Components `src/app/shared/layout/component/theme-configurator/theme-configurator.component.ts`	Configured OnPush change detection for signal-based theme configurator.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 Twelve components light,
OnPush shines so bright,
No needless checks today,
Input changes lead the way!
Performance hops with delight! 🚀✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding OnPush change detection strategy to components to reduce excessive re-renders and improve performance.
Description check	✅ Passed	The PR description comprehensively covers the template requirements with detailed context, profiling metrics, component-by-component justification, and explicitly lists excluded components.
Linked Issues check	✅ Passed	All code changes fully satisfy issue `#157` objectives: 12 safe presentational components migrated to OnPush, each verified for no unmanaged subscriptions or internal state mutations, with profiling demonstrating required CD cycle and time reductions.
Out of Scope Changes check	✅ Passed	All changes are strictly in-scope: only ChangeDetectionStrategy.OnPush metadata updates applied to the 12 components explicitly listed in issue `#157`, with no unrelated modifications or additional logic changes.
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 unit tests (beta)

Create PR with unit tests

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

balazs-szucs

🍻

* develop: fix(entrypoint): ensure books directory exists and is writable by the target user (grimmory-tools#119) fix(metadata): fix metadata fetching relying for filename "first" instead of better fields like ISBN in Goodreads/Bookdrop (grimmory-tools#85) chore(build): migrate Gradle build scripts from Groovy to Kotlin DSL (grimmory-tools#100) perf(ui): captures some low hanging fruit around excessive re-renders in the app (grimmory-tools#158) fix(api): check legacy tools path for binaries (grimmory-tools#146)

… in the app (grimmory-tools#158) Co-authored-by: Zack Yancey <yanceyz@proton.me>

… in the app (#158) Co-authored-by: Zack Yancey <yanceyz@proton.me>

perf(ui): captures some low hanging fruit around excessive re-renders…

54a3846

… in the app

balazs-szucs approved these changes Mar 24, 2026

View reviewed changes

balazs-szucs merged commit b964f41 into grimmory-tools:develop Mar 24, 2026
1 check passed

coderabbitai Bot mentioned this pull request Apr 9, 2026

fix(dashboard): improve performance with OnPush change detection #440

Merged

zachyale pushed a commit to zachyale/grimmory that referenced this pull request Apr 17, 2026

perf(ui): captures some low hanging fruit around excessive re-renders…

e6b8bd7

… in the app (grimmory-tools#158) Co-authored-by: Zack Yancey <yanceyz@proton.me>

zachyale pushed a commit to zachyale/grimmory that referenced this pull request Apr 17, 2026

perf(ui): captures some low hanging fruit around excessive re-renders…

e0eb5ba

… in the app (grimmory-tools#158) Co-authored-by: Zack Yancey <yanceyz@proton.me>

zachyale pushed a commit that referenced this pull request Apr 17, 2026

perf(ui): captures some low hanging fruit around excessive re-renders…

6da92f0

… in the app (#158) Co-authored-by: Zack Yancey <yanceyz@proton.me>

coderabbitai Bot mentioned this pull request Apr 21, 2026

fix(routes): update dashboard route to use dynamic component loading,optimize frontend loading #747

Closed

zachyale pushed a commit that referenced this pull request Apr 22, 2026

perf(ui): captures some low hanging fruit around excessive re-renders…

8778827

… in the app (#158) Co-authored-by: Zack Yancey <yanceyz@proton.me>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

perf(ui): captures some low hanging fruit around excessive re-renders in the app#158

perf(ui): captures some low hanging fruit around excessive re-renders in the app#158
balazs-szucs merged 1 commit into
grimmory-tools:developfrom
ZYancey:perf/onpush-presentational-components

ZYancey commented Mar 24, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Mar 24, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

balazs-szucs left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

ZYancey commented Mar 24, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

balazs-szucs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ZYancey commented Mar 24, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 24, 2026 •

edited

Loading