refactor(navbar-vertical): explicitly define change detection strategy#2201
refactor(navbar-vertical): explicitly define change detection strategy#2201spliffone wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates several vertical and primary navbar components, along with their test host components, to use ChangeDetectionStrategy.OnPush to improve performance. The feedback suggests applying ChangeDetectionStrategy.OnPush to SiNavbarItemComponent as well, rather than explicitly setting it to Default, to maintain consistency with the rest of the changes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| SiIconComponent | ||
| ], | ||
| templateUrl: './si-navbar-item.component.html', | ||
| changeDetection: ChangeDetectionStrategy.Default, |
There was a problem hiding this comment.
Consider using ChangeDetectionStrategy.OnPush instead of ChangeDetectionStrategy.Default for SiNavbarItemComponent to align with the project's performance standards and other components in this PR.
Since SiNavbarItemComponent uses signal-based inputs (item, quickAction) and signal queries, it is well-suited for OnPush. If there are any legacy non-signal properties or mutable objects being passed, you can inject ChangeDetectorRef and call markForCheck() where appropriate (e.g., inside ngDoCheck()).
| changeDetection: ChangeDetectionStrategy.Default, | |
| changeDetection: ChangeDetectionStrategy.OnPush, |
References
- Set changeDetection: ChangeDetectionStrategy.OnPush in @component decorator. (link)
08201f2 to
84cf601
Compare
Documentation.
Examples.
Dashboards Demo.
Playwright report.
Coverage Reports: