-
Notifications
You must be signed in to change notification settings - Fork 29
create a custom icon-button directive #3541
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughA new standalone Changes
Sequence Diagram(s)sequenceDiagram
actor User
User->>UiComponent: click profile button (app-icon-button)
UiComponent->>IconButtonComponent: buttonClick event emitted
IconButtonComponent-->>UiComponent: emits buttonClick
UiComponent->>UiComponent: closeSidenavOnMobile()
UiComponent->>Router: navigate('user-account')
Router-->>UiComponent: navigation complete
UiComponent-->>User: profile page shown
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
|
Deployed to https://pr-3541.aam-digital.net/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.ts (1)
27-28: Remove explicitstandalone: truesetting.The coding guidelines specify not to explicitly set
standalone: trueas it's the default behavior in this project.🔎 Proposed fix
@Component({ selector: "app-menu-item-list-editor", - standalone: true, imports: [AdminMenuItemComponent, DragDropModule, IconButtonComponent], templateUrl: "./menu-item-list-editor.component.html", styleUrl: "./menu-item-list-editor.component.scss", })Based on coding guidelines.
src/app/core/common-components/icon-button/icon-button.component.html (1)
1-62: Suggest reducing template duplication.The template repeats identical attribute bindings across four button variants. This duplication increases maintenance burden and the risk of inconsistencies.
Consider refactoring to extract common attributes or using a more dynamic approach:
🔎 Approach 1: Use ngSwitch on the wrapper and shared attributes
You could dynamically assign the Material button directive using attribute binding:
-@switch (buttonType()) { - @case ("mat-button") { - <button - mat-button - type="button" - [color]="color()" - [class]="cssClass()" - [attr.angulartics2On]="angulartics2On()" - [attr.angularticsCategory]="angularticsCategory()" - [attr.angularticsAction]="angularticsAction()" - (click)="onClick($event)" - > - <fa-icon [icon]="icon()" class="standard-icon-with-text"></fa-icon> - <ng-content></ng-content> - </button> - } - @case ("mat-raised-button") { - <button - mat-raised-button - type="button" - [color]="color()" - [class]="cssClass()" - [attr.angulartics2On]="angulartics2On()" - [attr.angularticsCategory]="angularticsCategory()" - [attr.angularticsAction]="angularticsAction()" - (click)="onClick($event)" - > - <fa-icon [icon]="icon()" class="standard-icon-with-text"></fa-icon> - <ng-content></ng-content> - </button> - } - @case ("mat-flat-button") { - <button - mat-flat-button - type="button" - [color]="color()" - [class]="cssClass()" - [attr.angulartics2On]="angulartics2On()" - [attr.angularticsCategory]="angularticsCategory()" - [attr.angularticsAction]="angularticsAction()" - (click)="onClick($event)" - > - <fa-icon [icon]="icon()" class="standard-icon-with-text"></fa-icon> - <ng-content></ng-content> - </button> - } - @default { - <button - mat-stroked-button - type="button" - [color]="color()" - [class]="cssClass()" - [attr.angulartics2On]="angulartics2On()" - [attr.angularticsCategory]="angularticsCategory()" - [attr.angularticsAction]="angularticsAction()" - (click)="onClick($event)" - > - <fa-icon [icon]="icon()" class="standard-icon-with-text"></fa-icon> - <ng-content></ng-content> - </button> - } -} +<button + [attr.mat-button]="buttonType() === 'mat-button' ? '' : null" + [attr.mat-raised-button]="buttonType() === 'mat-raised-button' ? '' : null" + [attr.mat-flat-button]="buttonType() === 'mat-flat-button' ? '' : null" + [attr.mat-stroked-button]="buttonType() === 'mat-stroked-button' || !buttonType() ? '' : null" + type="button" + [color]="color()" + [class]="cssClass()" + [attr.angulartics2On]="angulartics2On()" + [attr.angularticsCategory]="angularticsCategory()" + [attr.angularticsAction]="angularticsAction()" + (click)="onClick($event)" +> + <fa-icon [icon]="icon()" class="standard-icon-with-text"></fa-icon> + <ng-content></ng-content> +</button>Note: Verify this approach works correctly with Material button directives, as they may require compile-time resolution.
🔎 Approach 2: Use an inline template
Given the template's simplicity and per coding guidelines to "prefer inline templates for small components," consider moving this to an inline template in the component TypeScript file.
src/app/core/pwa-install/pwa-install.component.html (1)
7-9: Simplify static string bindings.The angulartics attributes are bound to static string literals using property binding syntax. Since these are constant strings, you can pass them directly as input values without the binding syntax:
🔎 Proposed simplification
- [angulartics2On]="'click'" - [angularticsCategory]="'UserAction'" - [angularticsAction]="'install_app'" + angulartics2On="click" + angularticsCategory="UserAction" + angularticsAction="install_app"This is clearer and avoids unnecessary property binding for static values.
src/app/core/common-components/icon-button/icon-button.component.ts (1)
21-32: Consider adding aria-label support for accessibility.While Material buttons provide baseline accessibility, there may be cases where the projected text content is insufficient for screen readers (e.g., when the text is very short or context-dependent). Consider adding an optional
ariaLabelinput:🔎 Proposed enhancement
Add to the component:
cssClass = input<string>(""); + ariaLabel = input<string | undefined>(undefined); angulartics2On = input<string | undefined>(undefined);Then in the template, add to each button element:
<button mat-button type="button" + [attr.aria-label]="ariaLabel()" [color]="color()"This allows consumers to provide explicit labels when needed, improving accessibility for complex UIs.
Based on learnings, all user-facing strings must be translatable and Angular Material accessibility features should be used in components.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.htmlsrc/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.tssrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.htmlsrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.tssrc/app/core/common-components/conditions-editor/conditions-editor.component.htmlsrc/app/core/common-components/conditions-editor/conditions-editor.component.tssrc/app/core/common-components/icon-button/icon-button.component.htmlsrc/app/core/common-components/icon-button/icon-button.component.scsssrc/app/core/common-components/icon-button/icon-button.component.spec.tssrc/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/pwa-install/pwa-install.component.htmlsrc/app/core/pwa-install/pwa-install.component.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.htmlsrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.ts
🧰 Additional context used
📓 Path-based instructions (15)
**/*.component.html
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.html: Use native control flow (@if,@for,@switch) instead of structural directives (*ngIf,*ngFor,*ngSwitch)
Use class bindings instead ofngClassdirective in Angular templates
Use style bindings instead ofngStyledirective in Angular templates
Keep templates simple and avoid complex logic in Angular templates
Use async pipe for observables in Angular templates instead of manual subscriptions
Files:
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.htmlsrc/app/core/pwa-install/pwa-install.component.htmlsrc/app/core/common-components/conditions-editor/conditions-editor.component.htmlsrc/app/core/common-components/icon-button/icon-button.component.htmlsrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.htmlsrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.html
**/*.component.{scss,html}
📄 CodeRabbit inference engine (AGENTS.md)
Use global style classes from
src/styles/globals/(e.g.,flex-row,margin-regular) instead of creating new custom styles
Files:
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.htmlsrc/app/core/pwa-install/pwa-install.component.htmlsrc/app/core/common-components/conditions-editor/conditions-editor.component.htmlsrc/app/core/common-components/icon-button/icon-button.component.scsssrc/app/core/common-components/icon-button/icon-button.component.htmlsrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.htmlsrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.html
**/*.component.{ts,html}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.{ts,html}: All user-facing strings must be translatable using Angular i18n markers or$localizein Aam Digital
Follow existing translation key patterns in Aam Digital
Use Angular Material accessibility features in components
Files:
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.htmlsrc/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.tssrc/app/core/pwa-install/pwa-install.component.htmlsrc/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/common-components/conditions-editor/conditions-editor.component.htmlsrc/app/core/common-components/icon-button/icon-button.component.htmlsrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.tssrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.htmlsrc/app/core/common-components/conditions-editor/conditions-editor.component.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.tssrc/app/core/pwa-install/pwa-install.component.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.html
**/*.component.{html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Follow WCAG guidelines for accessibility in Aam Digital
Files:
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.htmlsrc/app/core/pwa-install/pwa-install.component.htmlsrc/app/core/common-components/conditions-editor/conditions-editor.component.htmlsrc/app/core/common-components/icon-button/icon-button.component.scsssrc/app/core/common-components/icon-button/icon-button.component.htmlsrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.htmlsrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.html
**/*.component.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.ts: Use standalone components (default behavior, do NOT setstandalone: true)
Preferinput()andoutput()functions over@Inputand@Outputdecorators in Angular components
Use signals for reactive state management withcomputed()for derived state
SetchangeDetection: ChangeDetectionStrategy.OnPushfor all components
Prefer inline templates for small components in Angular
Prefer Reactive Forms over Template-driven forms in Angular
UseNgOptimizedImagefor static images instead of base64 encoded images
Use Angular Material components for UI consistency following Material Design guidelines
Implement OnPush change detection strategy for performance optimization
Use trackBy functions for lists in Angular templates for performance optimization
Files:
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.tssrc/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.tssrc/app/core/common-components/conditions-editor/conditions-editor.component.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.tssrc/app/core/pwa-install/pwa-install.component.ts
**/*.{component,service,directive,guard,interceptor,resolver,pipe}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use
inject()function instead of constructor injection for dependencies
Files:
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.tssrc/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.tssrc/app/core/common-components/conditions-editor/conditions-editor.component.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.tssrc/app/core/pwa-install/pwa-install.component.ts
**/*.{component,service}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{component,service}.ts: Useupdate()orset()instead ofmutate()when modifying signals in Angular
Create interfaces for configuration objects and let component classes implement them
Files:
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.tssrc/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.tssrc/app/core/common-components/conditions-editor/conditions-editor.component.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.tssrc/app/core/pwa-install/pwa-install.component.ts
**/*.{component,directive}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{component,directive}.ts: Do NOT use@HostBindingand@HostListenerdecorators; put host bindings inside thehostobject of@Componentor@Directivedecorator
Implement proper permissions checking via CASL integration usingEntityAbilityandDisableEntityOperationDirective
Files:
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.tssrc/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.tssrc/app/core/common-components/conditions-editor/conditions-editor.component.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.tssrc/app/core/pwa-install/pwa-install.component.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Use strict type checking with TypeScript and avoidanytype; useunknownwhen type is uncertain
Prefer type inference in TypeScript when obvious
Files:
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.tssrc/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.tssrc/app/core/common-components/conditions-editor/conditions-editor.component.tssrc/app/core/common-components/icon-button/icon-button.component.spec.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.tssrc/app/core/pwa-install/pwa-install.component.ts
**/*entity*.{service,component}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use proper generics for entity types in TypeScript
Files:
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.ts
src/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,js}: Use ESLint for linting (npm run lint)
Use Prettier for code formatting
Files:
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.tssrc/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.tssrc/app/core/common-components/conditions-editor/conditions-editor.component.tssrc/app/core/common-components/icon-button/icon-button.component.spec.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.tssrc/app/core/pwa-install/pwa-install.component.ts
src/app/core/common-components/**/*.component.ts
📄 CodeRabbit inference engine (AGENTS.md)
Shared components belong in
src/app/core/common-components/
Files:
src/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/common-components/conditions-editor/conditions-editor.component.ts
**/*.component.scss
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.scss: Use SCSS for styling with global variables and mixins fromsrc/styles/
If custom styles are necessary, create a class with meaningful name in component-specific SCSS file instead of inline styles
Files:
src/app/core/common-components/icon-button/icon-button.component.scss
**/*.{component,service}.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
Write unit tests for all new components and services using Jasmine/Karma
Files:
src/app/core/common-components/icon-button/icon-button.component.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.ts: Mock dependencies properly using Angular testing utilities in unit tests
Use established testing patterns from the Aam Digital project in unit tests
Files:
src/app/core/common-components/icon-button/icon-button.component.spec.ts
🧠 Learnings (23)
📓 Common learnings
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use Angular Material components for UI consistency following Material Design guidelines
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use Angular Material components for UI consistency following Material Design guidelines
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.htmlsrc/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.tssrc/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/common-components/conditions-editor/conditions-editor.component.htmlsrc/app/core/common-components/icon-button/icon-button.component.scsssrc/app/core/common-components/icon-button/icon-button.component.htmlsrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.tssrc/app/core/common-components/conditions-editor/conditions-editor.component.tssrc/app/core/common-components/icon-button/icon-button.component.spec.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.tssrc/app/core/pwa-install/pwa-install.component.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.{component,directive}.ts : Implement proper permissions checking via CASL integration using `EntityAbility` and `DisableEntityOperationDirective`
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*entity*.{service,component}.ts : Use proper generics for entity types in TypeScript
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*entity*.service.ts : Use entity services for data access and caching in Aam Digital
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{ts,html} : Use Angular Material accessibility features in components
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.tssrc/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/common-components/conditions-editor/conditions-editor.component.htmlsrc/app/core/common-components/icon-button/icon-button.component.scsssrc/app/core/common-components/icon-button/icon-button.component.htmlsrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.tssrc/app/core/common-components/conditions-editor/conditions-editor.component.tssrc/app/core/common-components/icon-button/icon-button.component.spec.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.tssrc/app/core/pwa-install/pwa-install.component.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Prefer `input()` and `output()` functions over `Input` and `Output` decorators in Angular components
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Implement OnPush change detection strategy for performance optimization
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.tssrc/app/core/common-components/conditions-editor/conditions-editor.component.tssrc/app/core/common-components/icon-button/icon-button.component.spec.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.tssrc/app/core/pwa-install/pwa-install.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to src/app/core/common-components/**/*.component.ts : Shared components belong in `src/app/core/common-components/`
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/common-components/icon-button/icon-button.component.scsssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.tssrc/app/core/pwa-install/pwa-install.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Prefer inline templates for small components in Angular
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/common-components/icon-button/icon-button.component.scsssrc/app/core/common-components/icon-button/icon-button.component.htmlsrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.tssrc/app/core/pwa-install/pwa-install.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.html : Use native control flow (`if`, `for`, `switch`) instead of structural directives (`*ngIf`, `*ngFor`, `*ngSwitch`)
Applied to files:
src/app/core/common-components/conditions-editor/conditions-editor.component.htmlsrc/app/core/common-components/icon-button/icon-button.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{scss,html} : Use global style classes from `src/styles/globals/` (e.g., `flex-row`, `margin-regular`) instead of creating new custom styles
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.scss
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.scss : Use SCSS for styling with global variables and mixins from `src/styles/`
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.scss
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use standalone components (default behavior, do NOT set `standalone: true`)
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.scsssrc/app/core/pwa-install/pwa-install.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{ts,html} : All user-facing strings must be translatable using Angular i18n markers or `$localize` in Aam Digital
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.scsssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.scss : If custom styles are necessary, create a class with meaningful name in component-specific SCSS file instead of inline styles
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.scss
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{html,scss} : Follow WCAG guidelines for accessibility in Aam Digital
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.scss
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.{component,service,directive,guard,interceptor,resolver,pipe}.ts : Use `inject()` function instead of constructor injection for dependencies
Applied to files:
src/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Prefer Reactive Forms over Template-driven forms in Angular
Applied to files:
src/app/core/common-components/conditions-editor/conditions-editor.component.tssrc/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.spec.ts : Mock dependencies properly using Angular testing utilities in unit tests
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.spec.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.{component,service}.spec.ts : Write unit tests for all new components and services using Jasmine/Karma
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.spec.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.spec.ts : Use established testing patterns from the Aam Digital project in unit tests
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.spec.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.html : Keep templates simple and avoid complex logic in Angular templates
Applied to files:
src/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.ts
🧬 Code graph analysis (2)
src/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.ts (1)
src/app/core/common-components/icon-button/icon-button.component.ts (1)
Component(13-39)
src/app/core/pwa-install/pwa-install.component.ts (1)
src/app/core/common-components/icon-button/icon-button.component.ts (1)
Component(13-39)
⏰ 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)
- GitHub Check: Seer Code Review
- GitHub Check: qa / test-e2e
- GitHub Check: qa / test-unit
- GitHub Check: build-webapp
🔇 Additional comments (17)
src/app/core/common-components/icon-button/icon-button.component.scss (1)
1-1: LGTM! Good use of global styles.The component correctly leverages global styles rather than creating custom component-specific styles, which aligns with the project's styling guidelines.
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.html (1)
22-31: LGTM! Proper migration to IconButtonComponent.The button correctly uses the new
app-icon-buttoncomponent with the(buttonClick)event binding, maintains the tooltip, and preserves i18n support.src/app/core/pwa-install/pwa-install.component.ts (2)
15-15: LGTM! Import statement is correct.The IconButtonComponent is properly imported from the common-components directory.
21-26: LGTM! Module imports correctly updated.The component's imports array properly includes IconButtonComponent while following the guideline to not explicitly set
standalone: true.src/app/core/common-components/conditions-editor/conditions-editor.component.ts (2)
23-23: LGTM! Import statement is correct.The IconButtonComponent is properly imported with the correct relative path.
32-41: LGTM! Module imports correctly updated.The imports array properly includes IconButtonComponent without explicitly setting
standalone: true, following the project's guidelines.src/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.ts (2)
13-13: LGTM! Import statement is correct.The IconButtonComponent is properly imported using the project's path alias.
21-21: LGTM! Module imports correctly updated.The component properly replaces FaIconComponent and MatButton with IconButtonComponent, following the project's guideline to not explicitly set
standalone: true.src/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.ts (1)
18-18: LGTM! Import statement is correct.The IconButtonComponent is properly imported with the correct relative path.
src/app/core/admin/admin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.ts (2)
36-36: LGTM! Import statement is correct.The IconButtonComponent is properly imported using the project's path alias.
38-51: LGTM! Module imports correctly updated.The component properly replaces FaIconComponent with IconButtonComponent in the imports array, following the project's guideline to not explicitly set
standalone: true.src/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.html (1)
2-11: LGTM! Proper migration to IconButtonComponent.The button correctly uses the new
app-icon-buttoncomponent with proper inputs (icon,buttonType,color,cssClass), the(buttonClick)event binding, and maintains i18n support.src/app/core/ui/menu-item-list-editor/menu-item-list-editor.component.html (1)
3-10: LGTM! Successful migration to IconButtonComponent.The button has been correctly migrated to use the new
app-icon-buttoncomponent with proper i18n handling and event binding via(buttonClick).src/app/core/common-components/conditions-editor/conditions-editor.component.html (1)
48-64: LGTM! Both buttons correctly migrated to IconButtonComponent.The "Add Condition" and "Edit JSON" buttons have been successfully migrated to use the new
app-icon-buttoncomponent with proper i18n markers and event bindings.src/app/core/pwa-install/pwa-install.component.html (1)
2-14: LGTM! Successful migration with analytics support.The PWA install button has been correctly migrated to use the new
app-icon-buttoncomponent with proper analytics tracking and i18n handling.src/app/core/common-components/icon-button/icon-button.component.spec.ts (1)
1-114: LGTM! Comprehensive test coverage for the new component.The test suite thoroughly covers all aspects of the IconButtonComponent including inputs, outputs, rendering, and attribute propagation. The tests follow Angular testing best practices using modern testing utilities.
src/app/core/common-components/icon-button/icon-button.component.ts (1)
1-39: LGTM! Component follows coding guidelines.The IconButtonComponent correctly implements the coding guidelines:
- Uses
input()andoutput()functions- Implements OnPush change detection
- Located in the appropriate shared components directory
- Proper type safety with well-defined union types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/app/core/common-components/icon-button/icon-button.component.ts (3)
10-12: Enhance JSDoc with parameter and event documentation.The JSDoc comment is minimal. Consider documenting the component's inputs, outputs, and usage example to improve developer experience.
🔎 Suggested JSDoc enhancement
/** * A reusable button component that combines an icon with text, + * centralizing the common pattern of Material buttons with FontAwesome icons. + * + * @example + * <app-icon-button icon="edit" buttonType="mat-stroked-button" (buttonClick)="onEdit()"> + * Edit data + * </app-icon-button> */
15-16: Consider using an inline template.The template is 50 lines but contains significant repetition. Per coding guidelines, small components should prefer inline templates. You could either inline the current template or refactor to reduce duplication first.
Based on coding guidelines, "Prefer inline templates for small components in Angular."
30-30: Consider usingoutput<void>()for the click event.The
Eventobject from the click is rarely needed by consumers. Usingoutput<void>()simplifies the API and makes it clear that no event details are provided.🔎 Simplified output signature
- buttonClick = output<Event>(); + buttonClick = output<void>(); - onClick(event: Event): void { - this.buttonClick.emit(event); + onClick(): void { + this.buttonClick.emit(); }src/app/core/common-components/icon-button/icon-button.component.html (1)
1-50: Reduce template duplication by consolidating button variants.The template repeats the same 12-line button structure four times, differing only in the
mat-*directive name (lines 3-12, 15-24, 27-36, 39-48). While the@switchcontrol flow is correct per modern Angular patterns, this duplication increases maintenance burden.However, the proposed refactoring using dynamic attribute binding (
[attr.mat-button]="...") will not work because Material button directives must be applied at compile time and cannot be set dynamically via attribute binding—this is an architectural constraint of Angular.Viable alternatives:
- Accept the current structure as the practical solution given Angular Material's constraints
- Extract button content into a separate presentational component if the duplication becomes difficult to maintain
- Consider using a single button element with different styling/classes based on
buttonType()if changing from Material directives is acceptableCurrent implementation using
@switchis appropriate.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/core/common-components/icon-button/icon-button.component.htmlsrc/app/core/common-components/icon-button/icon-button.component.spec.tssrc/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/pwa-install/pwa-install.component.html
💤 Files with no reviewable changes (1)
- src/app/core/pwa-install/pwa-install.component.html
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/core/common-components/icon-button/icon-button.component.spec.ts
🧰 Additional context used
📓 Path-based instructions (11)
**/*.component.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.ts: Use standalone components (default behavior, do NOT setstandalone: true)
Preferinput()andoutput()functions over@Inputand@Outputdecorators in Angular components
Use signals for reactive state management withcomputed()for derived state
SetchangeDetection: ChangeDetectionStrategy.OnPushfor all components
Prefer inline templates for small components in Angular
Prefer Reactive Forms over Template-driven forms in Angular
UseNgOptimizedImagefor static images instead of base64 encoded images
Use Angular Material components for UI consistency following Material Design guidelines
Implement OnPush change detection strategy for performance optimization
Use trackBy functions for lists in Angular templates for performance optimization
Files:
src/app/core/common-components/icon-button/icon-button.component.ts
**/*.{component,service,directive,guard,interceptor,resolver,pipe}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use
inject()function instead of constructor injection for dependencies
Files:
src/app/core/common-components/icon-button/icon-button.component.ts
**/*.{component,service}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{component,service}.ts: Useupdate()orset()instead ofmutate()when modifying signals in Angular
Create interfaces for configuration objects and let component classes implement them
Files:
src/app/core/common-components/icon-button/icon-button.component.ts
**/*.{component,directive}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{component,directive}.ts: Do NOT use@HostBindingand@HostListenerdecorators; put host bindings inside thehostobject of@Componentor@Directivedecorator
Implement proper permissions checking via CASL integration usingEntityAbilityandDisableEntityOperationDirective
Files:
src/app/core/common-components/icon-button/icon-button.component.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Use strict type checking with TypeScript and avoidanytype; useunknownwhen type is uncertain
Prefer type inference in TypeScript when obvious
Files:
src/app/core/common-components/icon-button/icon-button.component.ts
src/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,js}: Use ESLint for linting (npm run lint)
Use Prettier for code formatting
Files:
src/app/core/common-components/icon-button/icon-button.component.ts
**/*.component.{ts,html}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.{ts,html}: All user-facing strings must be translatable using Angular i18n markers or$localizein Aam Digital
Follow existing translation key patterns in Aam Digital
Use Angular Material accessibility features in components
Files:
src/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/common-components/icon-button/icon-button.component.html
src/app/core/common-components/**/*.component.ts
📄 CodeRabbit inference engine (AGENTS.md)
Shared components belong in
src/app/core/common-components/
Files:
src/app/core/common-components/icon-button/icon-button.component.ts
**/*.component.html
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.html: Use native control flow (@if,@for,@switch) instead of structural directives (*ngIf,*ngFor,*ngSwitch)
Use class bindings instead ofngClassdirective in Angular templates
Use style bindings instead ofngStyledirective in Angular templates
Keep templates simple and avoid complex logic in Angular templates
Use async pipe for observables in Angular templates instead of manual subscriptions
Files:
src/app/core/common-components/icon-button/icon-button.component.html
**/*.component.{scss,html}
📄 CodeRabbit inference engine (AGENTS.md)
Use global style classes from
src/styles/globals/(e.g.,flex-row,margin-regular) instead of creating new custom styles
Files:
src/app/core/common-components/icon-button/icon-button.component.html
**/*.component.{html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Follow WCAG guidelines for accessibility in Aam Digital
Files:
src/app/core/common-components/icon-button/icon-button.component.html
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use Angular Material components for UI consistency following Material Design guidelines
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Prefer inline templates for small components in Angular
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use Angular Material components for UI consistency following Material Design guidelines
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/common-components/icon-button/icon-button.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{ts,html} : Use Angular Material accessibility features in components
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/common-components/icon-button/icon-button.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to src/app/core/common-components/**/*.component.ts : Shared components belong in `src/app/core/common-components/`
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Implement OnPush change detection strategy for performance optimization
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/common-components/icon-button/icon-button.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Prefer `input()` and `output()` functions over `Input` and `Output` decorators in Angular components
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Prefer inline templates for small components in Angular
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/common-components/icon-button/icon-button.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Set `changeDetection: ChangeDetectionStrategy.OnPush` for all components
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.html : Use native control flow (`if`, `for`, `switch`) instead of structural directives (`*ngIf`, `*ngFor`, `*ngSwitch`)
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.html : Use style bindings instead of `ngStyle` directive in Angular templates
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.html : Keep templates simple and avoid complex logic in Angular templates
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Prefer Reactive Forms over Template-driven forms in Angular
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{ts,html} : All user-facing strings must be translatable using Angular i18n markers or `$localize` in Aam Digital
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.html : Use class bindings instead of `ngClass` directive in Angular templates
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.html
⏰ 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)
- GitHub Check: Seer Code Review
- GitHub Check: qa / test-unit
- GitHub Check: qa / test-e2e
- GitHub Check: build-webapp
🔇 Additional comments (1)
src/app/core/common-components/icon-button/icon-button.component.html (1)
10-10: Standard icon class is correctly applied.The
standard-icon-with-textclass is consistently applied to all icon instances, which properly implements the pattern mentioned in the PR objectives.Also applies to: 22-22, 34-34, 46-46
src/app/core/common-components/icon-button/icon-button.component.ts
Outdated
Show resolved
Hide resolved
sadaf895
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a well-structured and valuable refactor.
sleidig
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Abhinegi2 , did you search and replace all occurances across the code yet? (I guess almost anywhere we currently use class="standard-icon-with-text" is a candidate for refactoring?)
Maybe Copilot can do that here, so that we don't have the old implementations around, confusing developers on what to use.
...dmin-entity-details/admin-entity-panel-component/admin-entity-panel-component.component.html
Show resolved
Hide resolved
I’ve already searched all occurrences, and yes, there are still 4–5 places where this is not being used. I did try to update those as well, but for mat-flat-button, it’s not working as expected; only the icons are visible, and the text is not showing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @src/app/core/filter/filter/filter.component.html:
- Around line 8-16: The tooltip string is not marked for translation; replace
the property binding [matTooltip]="'Clear all filters'" with a static attribute
and an Angular i18n marker so the tooltip becomes translatable (e.g. use
matTooltip="Clear all filters" together with i18n-matTooltip on the
<app-icon-button> element). Update the <app-icon-button> in
filter.component.html (the element that currently sets [matTooltip]) to use
matTooltip plus i18n-matTooltip to mark the text for i18n.
In @src/app/core/filter/filter/filter.component.ts:
- Line 30: The imports array in filter.component.ts is currently inline;
reformat the imports array so each imported symbol (NgComponentOutlet,
FontAwesomeModule, MatButtonModule, MatTooltip, IconButtonComponent) appears on
its own line with a trailing comma, matching project style (one-per-line in the
imports: [...] array).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/core/dashboard/admin-dashboard/admin-dashboard.component.htmlsrc/app/core/dashboard/admin-dashboard/admin-dashboard.component.tssrc/app/core/filter/filter/filter.component.htmlsrc/app/core/filter/filter/filter.component.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.component.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.ts: Use standalone components (default behavior, do NOT setstandalone: true)
Preferinput()andoutput()functions over@Inputand@Outputdecorators in Angular components
Use signals for reactive state management withcomputed()for derived state
SetchangeDetection: ChangeDetectionStrategy.OnPushfor all components
Prefer inline templates for small components in Angular
Prefer Reactive Forms over Template-driven forms in Angular
UseNgOptimizedImagefor static images instead of base64 encoded images
Use Angular Material components for UI consistency following Material Design guidelines
Implement OnPush change detection strategy for performance optimization
Use trackBy functions for lists in Angular templates for performance optimization
Files:
src/app/core/dashboard/admin-dashboard/admin-dashboard.component.tssrc/app/core/filter/filter/filter.component.ts
**/*.{component,service,directive,guard,interceptor,resolver,pipe}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use
inject()function instead of constructor injection for dependencies
Files:
src/app/core/dashboard/admin-dashboard/admin-dashboard.component.tssrc/app/core/filter/filter/filter.component.ts
**/*.{component,service}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{component,service}.ts: Useupdate()orset()instead ofmutate()when modifying signals in Angular
Create interfaces for configuration objects and let component classes implement them
Files:
src/app/core/dashboard/admin-dashboard/admin-dashboard.component.tssrc/app/core/filter/filter/filter.component.ts
**/*.{component,directive}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{component,directive}.ts: Do NOT use@HostBindingand@HostListenerdecorators; put host bindings inside thehostobject of@Componentor@Directivedecorator
Implement proper permissions checking via CASL integration usingEntityAbilityandDisableEntityOperationDirective
Files:
src/app/core/dashboard/admin-dashboard/admin-dashboard.component.tssrc/app/core/filter/filter/filter.component.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Use strict type checking with TypeScript and avoidanytype; useunknownwhen type is uncertain
Prefer type inference in TypeScript when obvious
Files:
src/app/core/dashboard/admin-dashboard/admin-dashboard.component.tssrc/app/core/filter/filter/filter.component.ts
src/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,js}: Use ESLint for linting (npm run lint)
Use Prettier for code formatting
Files:
src/app/core/dashboard/admin-dashboard/admin-dashboard.component.tssrc/app/core/filter/filter/filter.component.ts
**/*.component.{ts,html}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.{ts,html}: All user-facing strings must be translatable using Angular i18n markers or$localizein Aam Digital
Follow existing translation key patterns in Aam Digital
Use Angular Material accessibility features in components
Files:
src/app/core/dashboard/admin-dashboard/admin-dashboard.component.tssrc/app/core/dashboard/admin-dashboard/admin-dashboard.component.htmlsrc/app/core/filter/filter/filter.component.tssrc/app/core/filter/filter/filter.component.html
**/*.component.html
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.html: Use native control flow (@if,@for,@switch) instead of structural directives (*ngIf,*ngFor,*ngSwitch)
Use class bindings instead ofngClassdirective in Angular templates
Use style bindings instead ofngStyledirective in Angular templates
Keep templates simple and avoid complex logic in Angular templates
Use async pipe for observables in Angular templates instead of manual subscriptions
Files:
src/app/core/dashboard/admin-dashboard/admin-dashboard.component.htmlsrc/app/core/filter/filter/filter.component.html
**/*.component.{scss,html}
📄 CodeRabbit inference engine (AGENTS.md)
Use global style classes from
src/styles/globals/(e.g.,flex-row,margin-regular) instead of creating new custom styles
Files:
src/app/core/dashboard/admin-dashboard/admin-dashboard.component.htmlsrc/app/core/filter/filter/filter.component.html
**/*.component.{html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Follow WCAG guidelines for accessibility in Aam Digital
Files:
src/app/core/dashboard/admin-dashboard/admin-dashboard.component.htmlsrc/app/core/filter/filter/filter.component.html
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use Angular Material components for UI consistency following Material Design guidelines
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Prefer inline templates for small components in Angular
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{ts,html} : Use Angular Material accessibility features in components
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use Angular Material components for UI consistency following Material Design guidelines
Applied to files:
src/app/core/dashboard/admin-dashboard/admin-dashboard.component.tssrc/app/core/dashboard/admin-dashboard/admin-dashboard.component.htmlsrc/app/core/filter/filter/filter.component.tssrc/app/core/filter/filter/filter.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to src/app/core/common-components/**/*.component.ts : Shared components belong in `src/app/core/common-components/`
Applied to files:
src/app/core/dashboard/admin-dashboard/admin-dashboard.component.tssrc/app/core/filter/filter/filter.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Implement OnPush change detection strategy for performance optimization
Applied to files:
src/app/core/filter/filter/filter.component.tssrc/app/core/filter/filter/filter.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{ts,html} : Use Angular Material accessibility features in components
Applied to files:
src/app/core/filter/filter/filter.component.tssrc/app/core/filter/filter/filter.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Prefer inline templates for small components in Angular
Applied to files:
src/app/core/filter/filter/filter.component.tssrc/app/core/filter/filter/filter.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.html : Use native control flow (`if`, `for`, `switch`) instead of structural directives (`*ngIf`, `*ngFor`, `*ngSwitch`)
Applied to files:
src/app/core/filter/filter/filter.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.{component,service}.ts : Use `update()` or `set()` instead of `mutate()` when modifying signals in Angular
Applied to files:
src/app/core/filter/filter/filter.component.html
🪛 GitHub Check: qa / code-quality
src/app/core/filter/filter/filter.component.ts
[failure] 30-30:
Replace NgComponentOutlet,·FontAwesomeModule,·MatButtonModule,·MatTooltip,·IconButtonComponent with ⏎····NgComponentOutlet,⏎····FontAwesomeModule,⏎····MatButtonModule,⏎····MatTooltip,⏎····IconButtonComponent,⏎··
⏰ 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)
- GitHub Check: Seer Code Review
- GitHub Check: qa / test-unit
- GitHub Check: qa / test-e2e
- GitHub Check: build-webapp
🔇 Additional comments (2)
src/app/core/dashboard/admin-dashboard/admin-dashboard.component.html (1)
79-87: LGTM! IconButtonComponent integration looks correct.The button replacement follows the expected pattern with proper event binding, icon input, and button configuration. The i18n marker is correctly applied to the text content.
src/app/core/dashboard/admin-dashboard/admin-dashboard.component.ts (1)
30-30: LGTM! IconButtonComponent import and registration look correct.The import statement and component registration in the imports array are properly configured, making IconButtonComponent available for use in the admin dashboard template.
Also applies to: 46-46
src/app/core/common-components/icon-button/icon-button.component.ts
Outdated
Show resolved
Hide resolved
src/app/core/dashboard/admin-dashboard/admin-dashboard.component.html
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/app/core/common-components/icon-button/icon-button.component.spec.ts (2)
7-90: Good test coverage with opportunity to enhance.The test suite follows Angular testing best practices and covers the main functionality. However, consider adding tests for content projection to address the mat-flat-button text visibility issue you mentioned in the PR comments.
💡 Suggested additional test for content projection
Add a test to verify that button text content is properly rendered and visible:
it("should project and display text content", () => { fixture.componentRef.setInput("icon", "edit"); fixture.componentRef.setInput("buttonType", "mat-flat-button"); fixture.detectChanges(); const buttonElement = fixture.nativeElement.querySelector("button"); const textContent = buttonElement.textContent.trim(); // Verify text is rendered (you'll need to add text via fixture or component template) expect(textContent).toBeTruthy(); expect(buttonElement.offsetHeight).toBeGreaterThan(0); });This would help catch the text visibility issue you encountered with mat-flat-button.
37-56: Consider testing all button type variants.The tests cover
mat-raised-buttonandmat-stroked-button, butmat-buttonandmat-flat-buttonare not tested. Given your reported issue withmat-flat-buttontext visibility, adding explicit tests for all variants would improve coverage.♻️ Suggested tests for remaining button types
it("should apply mat-button type", () => { fixture.componentRef.setInput("icon", "edit"); fixture.componentRef.setInput("buttonType", "mat-button"); fixture.detectChanges(); const buttonElement = fixture.nativeElement.querySelector("button[mat-button]"); expect(buttonElement).toBeTruthy(); }); it("should apply mat-flat-button type", () => { fixture.componentRef.setInput("icon", "edit"); fixture.componentRef.setInput("buttonType", "mat-flat-button"); fixture.detectChanges(); const buttonElement = fixture.nativeElement.querySelector("button[mat-flat-button]"); expect(buttonElement).toBeTruthy(); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.htmlsrc/app/core/common-components/conditions-editor/conditions-editor.component.htmlsrc/app/core/common-components/icon-button/icon-button.component.htmlsrc/app/core/common-components/icon-button/icon-button.component.spec.tssrc/app/core/common-components/icon-button/icon-button.component.tssrc/app/core/dashboard/admin-dashboard/admin-dashboard.component.htmlsrc/app/core/filter/filter/filter.component.htmlsrc/app/core/filter/filter/filter.component.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/app/core/dashboard/admin-dashboard/admin-dashboard.component.html
- src/app/core/filter/filter/filter.component.html
- src/app/core/common-components/conditions-editor/conditions-editor.component.html
- src/app/core/admin/setup-wizard/setup-wizard-button/setup-wizard-button.component.html
- src/app/core/common-components/icon-button/icon-button.component.html
- src/app/core/common-components/icon-button/icon-button.component.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.component.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.ts: Use standalone components (default behavior, do NOT setstandalone: true)
Preferinput()andoutput()functions over@Inputand@Outputdecorators in Angular components
Use signals for reactive state management withcomputed()for derived state
SetchangeDetection: ChangeDetectionStrategy.OnPushfor all components
Prefer inline templates for small components in Angular
Prefer Reactive Forms over Template-driven forms in Angular
UseNgOptimizedImagefor static images instead of base64 encoded images
Use Angular Material components for UI consistency following Material Design guidelines
Implement OnPush change detection strategy for performance optimization
Use trackBy functions for lists in Angular templates for performance optimization
Files:
src/app/core/filter/filter/filter.component.ts
**/*.{component,service,directive,guard,interceptor,resolver,pipe}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use
inject()function instead of constructor injection for dependencies
Files:
src/app/core/filter/filter/filter.component.ts
**/*.{component,service}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{component,service}.ts: Useupdate()orset()instead ofmutate()when modifying signals in Angular
Create interfaces for configuration objects and let component classes implement them
Files:
src/app/core/filter/filter/filter.component.ts
**/*.{component,directive}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{component,directive}.ts: Do NOT use@HostBindingand@HostListenerdecorators; put host bindings inside thehostobject of@Componentor@Directivedecorator
Implement proper permissions checking via CASL integration usingEntityAbilityandDisableEntityOperationDirective
Files:
src/app/core/filter/filter/filter.component.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Use strict type checking with TypeScript and avoidanytype; useunknownwhen type is uncertain
Prefer type inference in TypeScript when obvious
Files:
src/app/core/filter/filter/filter.component.tssrc/app/core/common-components/icon-button/icon-button.component.spec.ts
src/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,js}: Use ESLint for linting (npm run lint)
Use Prettier for code formatting
Files:
src/app/core/filter/filter/filter.component.tssrc/app/core/common-components/icon-button/icon-button.component.spec.ts
**/*.component.{ts,html}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.{ts,html}: All user-facing strings must be translatable using Angular i18n markers or$localizein Aam Digital
Follow existing translation key patterns in Aam Digital
Use Angular Material accessibility features in components
Files:
src/app/core/filter/filter/filter.component.ts
**/*.{component,service}.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
Write unit tests for all new components and services using Jasmine/Karma
Files:
src/app/core/common-components/icon-button/icon-button.component.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.ts: Mock dependencies properly using Angular testing utilities in unit tests
Use established testing patterns from the Aam Digital project in unit tests
Files:
src/app/core/common-components/icon-button/icon-button.component.spec.ts
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Prefer inline templates for small components in Angular
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use Angular Material components for UI consistency following Material Design guidelines
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use standalone components (default behavior, do NOT set `standalone: true`)
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{ts,html} : Use Angular Material accessibility features in components
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Implement OnPush change detection strategy for performance optimization
Applied to files:
src/app/core/filter/filter/filter.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use Angular Material components for UI consistency following Material Design guidelines
Applied to files:
src/app/core/filter/filter/filter.component.tssrc/app/core/common-components/icon-button/icon-button.component.spec.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Prefer inline templates for small components in Angular
Applied to files:
src/app/core/filter/filter/filter.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to src/app/core/common-components/**/*.component.ts : Shared components belong in `src/app/core/common-components/`
Applied to files:
src/app/core/filter/filter/filter.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{ts,html} : All user-facing strings must be translatable using Angular i18n markers or `$localize` in Aam Digital
Applied to files:
src/app/core/filter/filter/filter.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.{component,service,directive,guard,interceptor,resolver,pipe}.ts : Use `inject()` function instead of constructor injection for dependencies
Applied to files:
src/app/core/filter/filter/filter.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{ts,html} : Use Angular Material accessibility features in components
Applied to files:
src/app/core/filter/filter/filter.component.tssrc/app/core/common-components/icon-button/icon-button.component.spec.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.{component,service}.spec.ts : Write unit tests for all new components and services using Jasmine/Karma
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.spec.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.spec.ts : Mock dependencies properly using Angular testing utilities in unit tests
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.spec.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.spec.ts : Use established testing patterns from the Aam Digital project in unit tests
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.spec.ts
🧬 Code graph analysis (1)
src/app/core/filter/filter/filter.component.ts (1)
src/app/core/common-components/icon-button/icon-button.component.ts (1)
Component(13-34)
⏰ 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)
- GitHub Check: Seer Code Review
- GitHub Check: qa / test-e2e
- GitHub Check: qa / test-unit
- GitHub Check: build-webapp
🔇 Additional comments (2)
src/app/core/filter/filter/filter.component.ts (2)
13-13: LGTM: Unused import removed.The removal of
ActivatedRouteis appropriate as it's not used in the component logic.
22-22: LGTM: IconButtonComponent imported correctly.The import aligns with the PR's objective to adopt the new icon-button directive across the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/core/ui/ui/ui.component.html (1)
100-109: Migrate the Settings button toapp-icon-buttonfor consistency.The Settings button still uses the old pattern while the Profile and Sign out buttons below have been migrated to the new
app-icon-buttoncomponent. For consistency and to complete the migration objective, this button should also use the new pattern.♻️ Proposed refactor
- <button - (click)="closeSidenavOnMobile()" - mat-button - routerLink="/admin" - class="footer-cell admin-overview-button" - > - <fa-icon icon="wrench" class="standard-icon-with-text"></fa-icon> - <span i18n="Navigate to settings page">Settings</span> - </button> + <app-icon-button + (buttonClick)="closeSidenavOnMobile(); navigateToAdmin()" + icon="wrench" + class="footer-cell admin-overview-button" + i18n="Navigate to settings page" + > + Settings + </app-icon-button>Note: You'll need to add a
navigateToAdmin()method in the component to handle the navigation to/admin, similar to thenavigateToProfile()method used for the Profile button. Alternatively, ifapp-icon-buttonsupportsrouterLinkdirectly, you could use that instead.
🤖 Fix all issues with AI agents
In @src/app/core/ui/ui/ui.component.html:
- Around line 112-128: Add translator descriptions to the two footer icon
buttons: update the app-icon-button element with
(buttonClick)="navigateToProfile()" to include i18n="Navigate to user profile"
and update the app-icon-button element with (buttonClick)="logout()" to include
i18n="Log out of the application" so translators get context (follow the
Settings button pattern).
🧹 Nitpick comments (1)
src/app/core/ui/ui/ui.component.ts (1)
180-183: The implementation looks good.The method correctly closes the mobile sidenav before navigating to the authenticated user-account route, which is protected by AuthGuard. The flow is clear and follows existing patterns in the codebase.
Optional improvements:
- Consider extracting the route string to a constant for better maintainability.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/core/ui/ui/ui.component.htmlsrc/app/core/ui/ui/ui.component.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.component.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.ts: Use standalone components (default behavior, do NOT setstandalone: true)
Preferinput()andoutput()functions over@Inputand@Outputdecorators in Angular components
Use signals for reactive state management withcomputed()for derived state
SetchangeDetection: ChangeDetectionStrategy.OnPushfor all components
Prefer inline templates for small components in Angular
Prefer Reactive Forms over Template-driven forms in Angular
UseNgOptimizedImagefor static images instead of base64 encoded images
Use Angular Material components for UI consistency following Material Design guidelines
Implement OnPush change detection strategy for performance optimization
Use trackBy functions for lists in Angular templates for performance optimization
Files:
src/app/core/ui/ui/ui.component.ts
**/*.{component,service,directive,guard,interceptor,resolver,pipe}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use
inject()function instead of constructor injection for dependencies
Files:
src/app/core/ui/ui/ui.component.ts
**/*.{component,service}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{component,service}.ts: Useupdate()orset()instead ofmutate()when modifying signals in Angular
Create interfaces for configuration objects and let component classes implement them
Files:
src/app/core/ui/ui/ui.component.ts
**/*.{component,directive}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{component,directive}.ts: Do NOT use@HostBindingand@HostListenerdecorators; put host bindings inside thehostobject of@Componentor@Directivedecorator
Implement proper permissions checking via CASL integration usingEntityAbilityandDisableEntityOperationDirective
Files:
src/app/core/ui/ui/ui.component.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Use strict type checking with TypeScript and avoidanytype; useunknownwhen type is uncertain
Prefer type inference in TypeScript when obvious
Files:
src/app/core/ui/ui/ui.component.ts
src/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,js}: Use ESLint for linting (npm run lint)
Use Prettier for code formatting
Files:
src/app/core/ui/ui/ui.component.ts
**/*.component.{ts,html}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.{ts,html}: All user-facing strings must be translatable using Angular i18n markers or$localizein Aam Digital
Follow existing translation key patterns in Aam Digital
Use Angular Material accessibility features in components
Files:
src/app/core/ui/ui/ui.component.tssrc/app/core/ui/ui/ui.component.html
**/*.component.html
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.html: Use native control flow (@if,@for,@switch) instead of structural directives (*ngIf,*ngFor,*ngSwitch)
Use class bindings instead ofngClassdirective in Angular templates
Use style bindings instead ofngStyledirective in Angular templates
Keep templates simple and avoid complex logic in Angular templates
Use async pipe for observables in Angular templates instead of manual subscriptions
Files:
src/app/core/ui/ui/ui.component.html
**/*.component.{scss,html}
📄 CodeRabbit inference engine (AGENTS.md)
Use global style classes from
src/styles/globals/(e.g.,flex-row,margin-regular) instead of creating new custom styles
Files:
src/app/core/ui/ui/ui.component.html
**/*.component.{html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Follow WCAG guidelines for accessibility in Aam Digital
Files:
src/app/core/ui/ui/ui.component.html
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Prefer inline templates for small components in Angular
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use standalone components (default behavior, do NOT set `standalone: true`)
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use Angular Material components for UI consistency following Material Design guidelines
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use Angular Material components for UI consistency following Material Design guidelines
Applied to files:
src/app/core/ui/ui/ui.component.tssrc/app/core/ui/ui/ui.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.html : Use async pipe for observables in Angular templates instead of manual subscriptions
Applied to files:
src/app/core/ui/ui/ui.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{ts,html} : Use Angular Material accessibility features in components
Applied to files:
src/app/core/ui/ui/ui.component.tssrc/app/core/ui/ui/ui.component.html
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{ts,html} : All user-facing strings must be translatable using Angular i18n markers or `$localize` in Aam Digital
Applied to files:
src/app/core/ui/ui/ui.component.html
⏰ 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)
- GitHub Check: build-webapp
- GitHub Check: qa / test-unit
- GitHub Check: qa / test-e2e
- GitHub Check: Seer Code Review
🔇 Additional comments (2)
src/app/core/ui/ui/ui.component.ts (2)
26-26: LGTM!The import statement correctly references the new IconButtonComponent using the appropriate relative path.
82-82: LGTM!IconButtonComponent is correctly added to the component's imports array, enabling its use in the template.
sleidig
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the issue you mentioned in the menu. Reverted the use of the new icon button there 😢 Otherwise looks good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/app/core/common-components/icon-button/icon-button.component.scss:
- Around line 3-5: Remove the global forced width by deleting the button {
width: 100%; } rule in icon-button.component.scss so icon buttons no longer
expand to full width in flex containers; rely on parent layout or the existing
.full-width utility class when a full-width button is intentionally needed, and
ensure the component's template/class names (e.g., the icon-button component and
any references to .standard-icon-with-text) remain unchanged.
🧹 Nitpick comments (1)
src/app/core/common-components/icon-button/icon-button.component.scss (1)
1-1: Consider whether component-specific SCSS is necessary.Since the component leverages the global
.standard-icon-with-textclass for its styling, verify if this component-specific SCSS file is needed at all. If the global class provides all required styling, this file could potentially be removed.As per coding guidelines, prefer using global style classes from
src/styles/globals/instead of creating component-specific styles.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/core/common-components/icon-button/icon-button.component.scss
🧰 Additional context used
📓 Path-based instructions (3)
**/*.component.scss
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.scss: Use SCSS for styling with global variables and mixins fromsrc/styles/
If custom styles are necessary, create a class with meaningful name in component-specific SCSS file instead of inline styles
Files:
src/app/core/common-components/icon-button/icon-button.component.scss
**/*.component.{scss,html}
📄 CodeRabbit inference engine (AGENTS.md)
Use global style classes from
src/styles/globals/(e.g.,flex-row,margin-regular) instead of creating new custom styles
Files:
src/app/core/common-components/icon-button/icon-button.component.scss
**/*.component.{html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Follow WCAG guidelines for accessibility in Aam Digital
Files:
src/app/core/common-components/icon-button/icon-button.component.scss
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use Angular Material components for UI consistency following Material Design guidelines
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use standalone components (default behavior, do NOT set `standalone: true`)
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.html : Use class bindings instead of `ngClass` directive in Angular templates
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Prefer inline templates for small components in Angular
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use Angular Material components for UI consistency following Material Design guidelines
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.scss
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to src/app/core/common-components/**/*.component.ts : Shared components belong in `src/app/core/common-components/`
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.scss
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.scss : Use SCSS for styling with global variables and mixins from `src/styles/`
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.scss
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{ts,html} : Use Angular Material accessibility features in components
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.scss
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{scss,html} : Use global style classes from `src/styles/globals/` (e.g., `flex-row`, `margin-regular`) instead of creating new custom styles
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.scss
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Prefer inline templates for small components in Angular
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.scss
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use standalone components (default behavior, do NOT set `standalone: true`)
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.scss
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{html,scss} : Follow WCAG guidelines for accessibility in Aam Digital
Applied to files:
src/app/core/common-components/icon-button/icon-button.component.scss
⏰ 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). (5)
- GitHub Check: Seer Code Review
- GitHub Check: build-webapp
- GitHub Check: qa / test-unit
- GitHub Check: qa / code-quality
- GitHub Check: qa / test-e2e
| button { | ||
| width: 100%; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, confirm the file exists and read its content
echo "=== Reading icon-button.component.scss ==="
if [ -f "src/app/core/common-components/icon-button/icon-button.component.scss" ]; then
cat -n src/app/core/common-components/icon-button/icon-button.component.scss
else
echo "File not found at specified path, searching for it..."
fd icon-button.component.scss
fiRepository: Aam-Digital/ndb-core
Length of output: 238
🏁 Script executed:
#!/bin/bash
# Find all app-icon-button usages to assess layout contexts
echo "=== Finding app-icon-button usages ==="
rg -n 'app-icon-button' --type=html -A2 -B2
echo ""
echo "=== Checking how many usages exist ==="
rg -c 'app-icon-button' --type=html | wc -lRepository: Aam-Digital/ndb-core
Length of output: 9049
🏁 Script executed:
#!/bin/bash
# Check for global style classes that might be relevant
echo "=== Looking for global style classes ==="
if [ -d "src/styles" ]; then
find src/styles -type f \( -name "*.scss" -o -name "*.css" \) | head -20
echo ""
echo "=== Checking for global button or flex utility classes ==="
rg -n 'button|width|flex' src/styles/globals/ --type=scss
else
echo "src/styles directory not found"
fiRepository: Aam-Digital/ndb-core
Length of output: 950
🏁 Script executed:
#!/bin/bash
# Check global style classes for button and width utilities
echo "=== Checking _utility-classes.scss ==="
cat -n src/styles/globals/_utility-classes.scss
echo ""
echo "=== Checking _flex-classes.scss ==="
cat -n src/styles/globals/_flex-classes.scss
echo ""
echo "=== Checking _size-classes.scss ==="
cat -n src/styles/globals/_size-classes.scssRepository: Aam-Digital/ndb-core
Length of output: 8052
🏁 Script executed:
#!/bin/bash
# Search for standard-icon-with-text definition
echo "=== Searching for standard-icon-with-text class ==="
rg -n 'standard-icon-with-text' --type scss,html,css
echo ""
echo "=== Checking the icon-button component template ==="
cat -n src/app/core/common-components/icon-button/icon-button.component.htmlRepository: Aam-Digital/ndb-core
Length of output: 1632
🏁 Script executed:
#!/bin/bash
# Look at the specific usage contexts more carefully
echo "=== Examining flex-row with app-icon-button contexts ==="
rg -B5 'app-icon-button' --type html | grep -A5 -B5 'flex-row'Repository: Aam-Digital/ndb-core
Length of output: 2911
Remove the forced width: 100% rule as it breaks button layouts in flex containers.
The width: 100% rule on buttons forces every icon button to expand to full width, which causes layout issues when multiple buttons appear together in flex containers. Looking at actual usage across the codebase:
- conditions-editor.component.html (lines 48, 52): Two buttons in the same
flex-row gap-smallcontainer are forced to take full width instead of flowing naturally - filter.component.html (line 8): Button in
flex-row gap-regularunnecessarily takes full width - notification-rule.component.html (line 87): Button in
flex-row gap-regular flex-wrapis forced wider than needed
The component template shows the SCSS comment references "global .standard-icon-with-text class," but this class only styles the icon element itself (template line 9), not the button, and it's not part of a global styling system.
If full-width buttons are needed in specific contexts, use the existing .full-width utility class instead of forcing it globally. Remove this rule and let parent containers control button layout.
🤖 Prompt for AI Agents
In @src/app/core/common-components/icon-button/icon-button.component.scss around
lines 3 - 5, Remove the global forced width by deleting the button { width:
100%; } rule in icon-button.component.scss so icon buttons no longer expand to
full width in flex containers; rely on parent layout or the existing .full-width
utility class when a full-width button is intentionally needed, and ensure the
component's template/class names (e.g., the icon-button component and any
references to .standard-icon-with-text) remain unchanged.
|
🎉 This PR is included in version 3.68.0-master.13 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
to simplify use of icon + text closes #3458
|
🎉 This PR is included in version 3.68.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
PR Checklist
Before you request a manual PR review from someone, please go through all of this list, check the points and mark them checked here.
This helps us reduce the number of review cycles and use the time of our core reviewers more effectively.
ng lint) passesng test) passes⏪ if issues are commented from testing or code review, make changes. Please then re-check every item of the checklist here again.
Remaining TODOs:
Summary by CodeRabbit
New Features
Refactor
Behavior
✏️ Tip: You can customize this high-level summary in your review settings.