feat(side-nav): restructure component api #307 - #564
Conversation
BREAKING CHANGE: The <tedi-sidenav-*> element API is removed. Wrap items in <ul tedi-sidenav-list> and use the attribute forms instead: <tedi-sidenav-item> -> <li tedi-sidenav-item> (same for dropdown, dropdown-item, group-title). Dropdown groups now use [tedi-sidenav-dropdown-group-parent] + <ul tedi-sidenav-dropdown-group-list>.
📝 WalkthroughWalkthroughThe SideNav now uses semantic ChangesSideNav semantic composition
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SideNavDropdownGroupComponent
participant SideNavService
participant SideNavComponent
User->>SideNavDropdownGroupComponent: Toggle non-link group
SideNavDropdownGroupComponent->>SideNavService: setOpenGroup(group)
SideNavService->>SideNavComponent: Update mobile group state
User->>SideNavComponent: Select back-to-parent
SideNavComponent->>SideNavService: handleBackToParentMenu()
SideNavService->>SideNavDropdownGroupComponent: Close open group
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
GitHub Actions
Inactive
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
tedi/components/layout/sidenav/sidenav.component.scss (4)
299-312: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGroup-title typography duplicated verbatim.
The padding/font-size/weight/line-height/color/text-transform values here exactly repeat
.tedi-sidenav-group-title__text(lines 33-41) and its&--mobileoverride (45-49). Extract a shared placeholder (%group-title-textor a mixin) to avoid the two blocks drifting apart on future redesigns.♻️ Suggested extraction
+%sidenav-group-title-text { + padding: var(--layout-grid-gutters-08) + var(--navigation-vertical-item-padding-right) + var(--layout-grid-gutters-08) + var(--navigation-vertical-item-padding-left-level-1); + font-size: var(--body-extra-small-bold-size); + font-weight: var(--body-extra-small-bold-weight); + line-height: var(--body-extra-small-bold-line-height); + color: var(--navigation-vertical-group-title-text); + text-transform: uppercase; +}Then
@extend %sidenav-group-title-text;in both the&--mobile .tedi-sidenav-group-title__textrule and the&-group-open ... .tedi-sidenav-dropdown-item__triggerrule.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tedi/components/layout/sidenav/sidenav.component.scss` around lines 299 - 312, Extract the duplicated group-title typography and spacing declarations into a shared placeholder or mixin near the existing sidenav group-title styles. Reuse it in both the mobile `.tedi-sidenav-group-title__text` rule and the open dropdown trigger selector `.tedi-sidenav-dropdown-group--open ... .tedi-sidenav-dropdown-item__trigger`, preserving each rule’s intended overrides.
233-262: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSelector nesting exceeds the 4-level max.
.tedi-sidenav → &--mobile → .tedi-sidenav-dropdown-group--parent-plain → .tedi-sidenav-dropdown-group__parent--drillable → &:hover/&:focus-visibleis 5 brace levels deep. Consider flattening, e.g. compound the drillable class directly with&:hover/&:focus-visibleat the previous level, or extract a dedicated top-level selector for.tedi-sidenav-dropdown-group__parent--drillable.As per coding guidelines: "Keep component SCSS nesting depth at a maximum of 4 levels."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tedi/components/layout/sidenav/sidenav.component.scss` around lines 233 - 262, Flatten the hover and focus-visible selectors inside .tedi-sidenav-dropdown-group--parent-plain so the drillable parent state does not exceed the four-level SCSS nesting limit. Preserve the existing display, alignment, cursor, trigger, caret, hover, focus-ring, and closed-list styles while restructuring only the selector nesting.Source: Coding guidelines
517-544: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTree-connector styling block duplicates lines 488-516 almost verbatim.
Trigger padding-left calc,
::before/::afterdimensions and colors, and the:last-of-type::before { height: 50% }rule are repeated here identically for a different selector context. Extract a shared placeholder selector to keep both connector variants in sync.♻️ Suggested extraction
+%sidenav-collapsed-tree-connector-neutral { + .tedi-sidenav-dropdown-item__trigger { + padding-left: calc( + var(--dropdown-item-padding-x) + var(--_sidenav-tree-container) + + var(--_gap) + ); + } + + &::after { + top: 50%; + width: var(--_sidenav-tree-branch-width); + height: var(--_sidenav-tree-trunk-width); + background-color: var(--navigation-vertical-tree-neutral-default); + transform: translateY(-50%); + } + + &::before { + width: var(--_sidenav-tree-trunk-width); + height: 100%; + background-color: var(--navigation-vertical-tree-neutral-default); + } + + &:last-of-type::before { + height: 50%; + } +}Then
@extend %sidenav-collapsed-tree-connector-neutral;in both the.tedi-sidenav-dropdown-group__list > .tedi-sidenav-dropdown-itemrule and the nested-dropdown:has()rule.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tedi/components/layout/sidenav/sidenav.component.scss` around lines 517 - 544, Extract the duplicated tree-connector declarations from the existing connector blocks into a shared placeholder named %sidenav-collapsed-tree-connector-neutral, then extend it from both .tedi-sidenav-dropdown-group__list > .tedi-sidenav-dropdown-item and the nested .tedi-sidenav-dropdown:has(> .tedi-sidenav-dropdown-item--parent) > .tedi-sidenav-dropdown-item:not(.tedi-sidenav-dropdown-item--parent) rule. Preserve the current padding, pseudo-element styling, colors, and last-of-type height behavior.
178-182: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMerge the sidenav hard-coded
3.5remoffsets with the shared header-height token.
--layout-header-heightis already the source of truth for the header height and is reused by the header profile offset, so avoid duplicating3.5remin the sidenav overlay inset and the mobile min-height calculation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tedi/components/layout/sidenav/sidenav.component.scss` around lines 178 - 182, Replace the hard-coded 3.5rem offset in the mobile min-height calculation near the sidenav `&--mobile` styles with the shared `--layout-header-height` token. Also update the sidenav overlay inset in `tedi/components/layout/sidenav/sidenav-overlay/sidenav-overlay.component.scss` at lines 6-9 to use that token; no other changes are needed.tedi/components/layout/sidenav/sidenav.component.ts (1)
114-129: 📐 Maintainability & Code Quality | 🔵 TrivialReaching into a sibling component's private field via bracket notation.
this.sidenavService.openGroup()?.["host"]?.nativeElementaccessesSideNavDropdownGroupComponent's privatehostfield through TypeScript's bracket-notation escape hatch. It mirrors the existinghandleBackToMainMenu()pattern, so it's consistent, but both are relying on an unintended TS loophole rather than a real public contract.SideNavDropdownComponentalready exposes a properelement()getter for the same purpose — exposing an equivalent public accessor onSideNavDropdownGroupComponentwould remove the reliance on private-field poking.
[recommended_refactor:low_effort_and_high_reward]♻️ Suggested direction
// sidenav-dropdown-group.component.ts + element(): HTMLElement { + return this.host.nativeElement; + } // sidenav.component.ts - const groupEl = this.sidenavService.openGroup()?.["host"] - ?.nativeElement as HTMLElement | undefined; + const groupEl = this.sidenavService.openGroup()?.element();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tedi/components/layout/sidenav/sidenav.component.ts` around lines 114 - 129, Replace the private-field bracket access in handleBackToParentMenu with a public element accessor on SideNavDropdownGroupComponent, adding an equivalent element() getter if needed. Use that accessor to obtain the group’s native HTMLElement before calling sidenavService.handleBackToParentMenu, and preserve the existing focus behavior.tedi/components/layout/sidenav/sidenav-item/sidenav-item.component.ts (1)
115-122: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSeeding
defaultOpenviaafterNextRendercauses a visible closed→open flash on first paint.Since
dropdownis a@ContentChild, it is already available inngAfterContentInit(which runs before the view's first render), so settingdropdown.open.set(true)there — instead of deferring toafterNextRenderinngAfterViewInit— would let the initial render already reflect the open state. Angular's own NG0100 guidance recommendsngAfterContentInitfor content-derived bindings precisely to avoid this kind of post-render flip.♻️ Suggested approach
-export class SideNavItemComponent implements AfterViewInit, OnInit, OnDestroy { +export class SideNavItemComponent implements AfterViewInit, AfterContentInit, OnInit, OnDestroy { ... + ngAfterContentInit(): void { + if (this.dropdown && this.defaultOpen()) { + this.dropdown.open.set(true); + } + } + ngAfterViewInit(): void { ... - // Seed the initial expanded state after render to avoid changing a - // just-checked binding during the same change-detection pass. - if (this.defaultOpen()) { - afterNextRender(() => dropdown.open.set(true), { - injector: this.injector, - }); - } - this.eventListeners.push(...)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tedi/components/layout/sidenav/sidenav-item/sidenav-item.component.ts` around lines 115 - 122, Move the default-open initialization from the afterNextRender callback in ngAfterViewInit to ngAfterContentInit, setting the `@ContentChild` dropdown.open signal before the first render. Preserve the defaultOpen condition and remove the deferred afterNextRender logic and any now-unused dependencies.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/tedi-angular/references/components.md`:
- Around line 1533-1553: Update the tedi-sidenav-dropdown-group-parent anchor in
the SideNav example to use href instead of route, leaving route bindings on
supported SideNavItemComponent and SideNavDropdownItemComponent elements
unchanged.
In `@tedi/components/layout/sidenav/sidenav-composition.spec.ts`:
- Around line 85-95: Add the project’s TediTranslationService mock to the
TestBed providers in the beforeEach setup for NativeHost, alongside the existing
TEDI_TRANSLATION_DEFAULT_TOKEN provider. Keep the default locale token and
existing SideNavService configuration unchanged.
In
`@tedi/components/layout/sidenav/sidenav-dropdown-group/sidenav-dropdown-group.component.html`:
- Around line 12-14: Update the caret tedi-icon in the isDrillable() block to
use color="inherit" instead of the hardcoded white color, so it follows the
parent foreground color and conforms to the component-owned icon guideline.
In `@tedi/components/layout/sidenav/sidenav.component.ts`:
- Around line 39-45: The legacy-item handling is inconsistent: hasLegacyItems is
defined but the promised wrapper is absent. Update sidenav.component.html to use
hasLegacyItems and render a tedi-sidenav-list ul around legacy projected items,
preserving direct projection for consumer-provided lists; update the related
sidenav.component.ts comment only as needed to match the implemented behavior.
---
Nitpick comments:
In `@tedi/components/layout/sidenav/sidenav-item/sidenav-item.component.ts`:
- Around line 115-122: Move the default-open initialization from the
afterNextRender callback in ngAfterViewInit to ngAfterContentInit, setting the
`@ContentChild` dropdown.open signal before the first render. Preserve the
defaultOpen condition and remove the deferred afterNextRender logic and any
now-unused dependencies.
In `@tedi/components/layout/sidenav/sidenav.component.scss`:
- Around line 299-312: Extract the duplicated group-title typography and spacing
declarations into a shared placeholder or mixin near the existing sidenav
group-title styles. Reuse it in both the mobile
`.tedi-sidenav-group-title__text` rule and the open dropdown trigger selector
`.tedi-sidenav-dropdown-group--open ... .tedi-sidenav-dropdown-item__trigger`,
preserving each rule’s intended overrides.
- Around line 233-262: Flatten the hover and focus-visible selectors inside
.tedi-sidenav-dropdown-group--parent-plain so the drillable parent state does
not exceed the four-level SCSS nesting limit. Preserve the existing display,
alignment, cursor, trigger, caret, hover, focus-ring, and closed-list styles
while restructuring only the selector nesting.
- Around line 517-544: Extract the duplicated tree-connector declarations from
the existing connector blocks into a shared placeholder named
%sidenav-collapsed-tree-connector-neutral, then extend it from both
.tedi-sidenav-dropdown-group__list > .tedi-sidenav-dropdown-item and the nested
.tedi-sidenav-dropdown:has(> .tedi-sidenav-dropdown-item--parent) >
.tedi-sidenav-dropdown-item:not(.tedi-sidenav-dropdown-item--parent) rule.
Preserve the current padding, pseudo-element styling, colors, and last-of-type
height behavior.
- Around line 178-182: Replace the hard-coded 3.5rem offset in the mobile
min-height calculation near the sidenav `&--mobile` styles with the shared
`--layout-header-height` token. Also update the sidenav overlay inset in
`tedi/components/layout/sidenav/sidenav-overlay/sidenav-overlay.component.scss`
at lines 6-9 to use that token; no other changes are needed.
In `@tedi/components/layout/sidenav/sidenav.component.ts`:
- Around line 114-129: Replace the private-field bracket access in
handleBackToParentMenu with a public element accessor on
SideNavDropdownGroupComponent, adding an equivalent element() getter if needed.
Use that accessor to obtain the group’s native HTMLElement before calling
sidenavService.handleBackToParentMenu, and preserve the existing focus behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 98cc1ec7-c062-4a8b-ab5a-4b2b317e989a
📒 Files selected for processing (36)
skills/tedi-angular/references/components.mdsrc/styles/cdk.scsstedi/components/layout/header/header-profile/header-profile.component.scsstedi/components/layout/header/header.stories.tstedi/components/layout/sidenav/index.tstedi/components/layout/sidenav/sidenav-composition.spec.tstedi/components/layout/sidenav/sidenav-dropdown-group/sidenav-dropdown-group-list.component.tstedi/components/layout/sidenav/sidenav-dropdown-group/sidenav-dropdown-group-parent.directive.tstedi/components/layout/sidenav/sidenav-dropdown-group/sidenav-dropdown-group.component.htmltedi/components/layout/sidenav/sidenav-dropdown-group/sidenav-dropdown-group.component.scsstedi/components/layout/sidenav/sidenav-dropdown-group/sidenav-dropdown-group.component.spec.tstedi/components/layout/sidenav/sidenav-dropdown-group/sidenav-dropdown-group.component.tstedi/components/layout/sidenav/sidenav-dropdown-item/sidenav-dropdown-item.component.htmltedi/components/layout/sidenav/sidenav-dropdown-item/sidenav-dropdown-item.component.scsstedi/components/layout/sidenav/sidenav-dropdown-item/sidenav-dropdown-item.component.spec.tstedi/components/layout/sidenav/sidenav-dropdown-item/sidenav-dropdown-item.component.tstedi/components/layout/sidenav/sidenav-dropdown/sidenav-dropdown.component.htmltedi/components/layout/sidenav/sidenav-dropdown/sidenav-dropdown.component.scsstedi/components/layout/sidenav/sidenav-dropdown/sidenav-dropdown.component.spec.tstedi/components/layout/sidenav/sidenav-dropdown/sidenav-dropdown.component.tstedi/components/layout/sidenav/sidenav-group-title/sidenav-group-title.component.scsstedi/components/layout/sidenav/sidenav-group-title/sidenav-group-title.component.tstedi/components/layout/sidenav/sidenav-item/sidenav-item.component.htmltedi/components/layout/sidenav/sidenav-item/sidenav-item.component.scsstedi/components/layout/sidenav/sidenav-item/sidenav-item.component.spec.tstedi/components/layout/sidenav/sidenav-item/sidenav-item.component.tstedi/components/layout/sidenav/sidenav-list/sidenav-list.component.tstedi/components/layout/sidenav/sidenav-overlay/sidenav-overlay.component.scsstedi/components/layout/sidenav/sidenav.component.htmltedi/components/layout/sidenav/sidenav.component.scsstedi/components/layout/sidenav/sidenav.component.spec.tstedi/components/layout/sidenav/sidenav.component.tstedi/components/layout/sidenav/sidenav.stories.tstedi/services/sidenav/sidenav.service.spec.tstedi/services/sidenav/sidenav.service.tstedi/services/translation/translations.ts
💤 Files with no reviewable changes (2)
- tedi/components/layout/sidenav/sidenav-group-title/sidenav-group-title.component.scss
- tedi/components/layout/sidenav/sidenav-dropdown/sidenav-dropdown.component.scss
GitHub Actions
Inactive
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tedi/components/layout/sidenav/sidenav-item/sidenav-item.component.ts`:
- Around line 102-108: Remove the dropdown.open update from ngAfterContentInit;
initialize the expanded state through earlier controlled state/deep props before
rendering, or defer the update with afterNextRender/next-tick scheduling so it
does not change a checked binding during the hook.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f268e6e5-bb61-4335-8883-a4d1d1ec796d
📒 Files selected for processing (12)
skills/tedi-angular/references/components.mdtedi/components/layout/sidenav/sidenav-dropdown-group/sidenav-dropdown-group.component.htmltedi/components/layout/sidenav/sidenav-dropdown-group/sidenav-dropdown-group.component.tstedi/components/layout/sidenav/sidenav-dropdown-item/sidenav-dropdown-item.component.scsstedi/components/layout/sidenav/sidenav-item/sidenav-item.component.scsstedi/components/layout/sidenav/sidenav-item/sidenav-item.component.spec.tstedi/components/layout/sidenav/sidenav-item/sidenav-item.component.tstedi/components/layout/sidenav/sidenav.component.htmltedi/components/layout/sidenav/sidenav.component.scsstedi/components/layout/sidenav/sidenav.component.spec.tstedi/components/layout/sidenav/sidenav.component.tstedi/components/layout/sidenav/sidenav.stories.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- tedi/components/layout/sidenav/sidenav-item/sidenav-item.component.spec.ts
- skills/tedi-angular/references/components.md
- tedi/components/layout/sidenav/sidenav.component.html
- tedi/components/layout/sidenav/sidenav-dropdown-group/sidenav-dropdown-group.component.html
- tedi/components/layout/sidenav/sidenav-dropdown-group/sidenav-dropdown-group.component.ts
- tedi/components/layout/sidenav/sidenav-item/sidenav-item.component.scss
- tedi/components/layout/sidenav/sidenav.component.scss
- tedi/components/layout/sidenav/sidenav.stories.ts
| ngAfterContentInit(): void { | ||
| // Seed the initial expanded state before the first render so the dropdown | ||
| // paints open without changing a just-checked binding. | ||
| if (this.defaultOpen() && this.dropdown) { | ||
| this.dropdown.open.set(true); | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file outline =="
ast-grep outline tedi/components/layout/sidenav/sidenav-item/sidenav-item.component.ts --view expanded || true
echo "== relevant component lines =="
cat -n tedi/components/layout/sidenav/sidenav-item/sidenav-item.component.ts | sed -n '1,180p'
echo "== locate dropdown component files =="
fd -a 'sidenav-dropdown|sidenav-item' . | sed 's#^\./##'
echo "== inspect dropdown signal definition if present =="
while IFS= read -r f; do
echo "--- $f"
rg -n "open|defaultOpen|ngAfterContentInit|ngAfterViewInit|AfterContentInit|AfterViewInit" "$f" || true
done < <(fd 'sidenav-dropdown.*\.(ts|html|scss)$' .)Repository: TEDI-Design-System/angular
Length of output: 1938
🌐 Web query:
Angular docs ngAfterContentInit state changes ExpressionChangedAfterItHasBeenCheckedError lifecycle
💡 Result:
The ExpressionChangedAfterItHasBeenCheckedError occurs in Angular development mode when an expression in a component's template changes its value after the change detection cycle has already completed [1][2]. Angular runs an additional verification pass after the main change detection cycle to ensure that bindings remain stable; if a property used in the template is modified during this verification phase, the error is thrown to prevent inconsistent UI states [1][3]. When using the ngAfterContentInit lifecycle hook, you may encounter this error if you attempt to update component properties that are bound to your template [4]. Because ngAfterContentInit is called after Angular has initialized the projected content but during the same overall lifecycle process, modifying state here can trigger the mismatch between the initial change detection run and the subsequent verification check [5][4]. To resolve or avoid this error, consider the following strategies: 1. Asynchronous Updates: Wrap the property update in a setTimeout or use a Promise/Observable to defer the change until the next JavaScript turn [5][6]. This ensures the current change detection cycle finishes before the update occurs. 2. Refactor Timing: Move the state initialization or updates to an earlier lifecycle hook, such as ngOnInit, where it is safer to perform initial data setup [1][7]. 3. Ensure Unidirectional Data Flow: Avoid updating parent or template-bound properties from child components within lifecycle hooks that run after the view/content has been checked [1][2]. The error is specific to development mode and is designed to enforce unidirectional data flow, ensuring that data changes originate from predictable sources rather than side effects during the change detection process [1][2].
Citations:
- 1: https://angular.dev/errors/NG0100
- 2: https://angular.love/expression-changed-error-overview
- 3: https://angular.love/deep-dive-into-the-infamous-expressionchangedafterithasbeencheckederror-in-angular
- 4: https://angular.dev/guide/components/lifecycle
- 5: https://stackoverflow.com/questions/45591545/got-expressionchangedafterithasbeencheckederror-when-using-ngfor-and-contentchi
- 6: https://stackoverflow.com/questions/34364880/expression-has-changed-after-it-was-checked
- 7: https://blog.angular-university.io/angular-debugging/
🌐 Web query:
Angular lifecycle ngAfterContentInit after first change detection cycle signal update safe default open
💡 Result:
Updating a signal directly inside ngAfterContentInit is generally not considered safe and will typically trigger an NG0100: ExpressionChangedAfterItHasBeenCheckedError in development mode [1][2][3]. This error occurs because Angular's change detection has already completed for that phase, and modifying a value used in the view after that point violates the expected data flow [4][2]. To perform updates safely after content initialization, consider the following strategies: 1. Defer the Update: If you must update state based on content children, use afterNextRender or wrap the update in a microtask (e.g., Promise.resolve.then(...)) to ensure the update happens after the current change detection cycle has stabilized [5][6][7]. 2. Use Reactive Alternatives: If possible, avoid lifecycle hooks for state management entirely. Use computed signals to derive state reactively from your content query signals [8][9][10]. This allows Angular to manage the dependency graph automatically, preventing the error by keeping updates within the reactive system rather than imperative lifecycle methods [9][10]. 3. Evaluate Use Case: Re-examine if the state modification truly needs to happen in ngAfterContentInit. If the value is needed for initial rendering, it should ideally be handled earlier in the component lifecycle (e.g., in the constructor or ngOnInit) or via reactive derivation [5][7][2]. Effects are not recommended for propagating state or performing these types of updates, as they can also trigger NG0100 if used to modify signals directly [11][9]. In modern Angular, rely on signals and computed properties to handle derived state, and reserve lifecycle hooks or afterNextRender for genuine side effects that interact with the DOM or external systems [5][6][7].
Citations:
- 1: https://angular.dev/guide/components/lifecycle
- 2: https://angular.dev/errors/NG0100
- 3: https://github.com/angular/angular/blob/main/adev/src/content/reference/errors/NG0100.md
- 4: ExpressionChangedAfterItHasBeenCheckedError when setting value of signal in
ngOnInitangular/angular#54197 - 5: https://arc.dev/employer-blog/angular-lifecycle-hooks-a-practical-guide-with-real-component-patterns/
- 6: https://dev.to/cristiansifuentes/angular-20-mastering-the-component-lifecycle-zoneless-ready-2025-edition-17b6
- 7: https://code-js.in/angular/angular-component-lifecycle-hooks-2026/
- 8: contentChildren signal value changes before child components are initialised angular/angular#56224
- 9: https://v17.angular.io/guide/signals
- 10: https://www.resumelens.org/blog/angular/angular-lifecycle-hooks
- 11: https://github.com/angular/skills/blob/HEAD/angular-developer/references/effects.md
🌐 Web query:
Angular lifecycle ngAfterContentInit runs after first change detection phase content children bound signal
💡 Result:
In the Angular component lifecycle, the ngAfterContentInit hook is called once during the initial change detection phase, specifically after the component's projected content (content children) has been fully initialized [1][2][3]. When using legacy content queries (@ContentChild or @ContentChildren), Angular guarantees that these queries are populated and ready for access by the time ngAfterContentInit runs [4][5][6]. However, the behavior differs significantly when using modern signal-based content queries (contentChild or contentChildren) [7][8]. Because signal queries are reactive, they do not rely on the legacy ngAfterContentInit hook for initialization access [8]. If you attempt to use an effect to initialize logic based on content children accessed via signal queries, be aware of the following: 1. Timing Guarantees: Unlike the explicit timing guarantee provided by ngAfterContentInit for legacy queries, signal-based content queries do not guarantee that the initialization logic of the projected child components has already completed when the contentChildren signal is first accessed [7]. 2. Reactivity: Because signal queries update reactively, they often eliminate the need for the ngAfterContentInit lifecycle hook entirely [8]. 3. Best Practice: If your initialization logic depends on signals within the content children, ensure those dependencies are correctly tracked within a reactive context (like a computed or effect) or continue using ngOnInit in the child components to ensure their internal state is initialized before the parent component interacts with them [7]. In summary, while ngAfterContentInit remains a lifecycle event that runs after content initialization [1][3], it is increasingly superseded by signal-based reactive patterns that resolve content dependencies automatically as they change, rather than at a single, fixed point in time [8].
Citations:
- 1: https://angular.dev/guide/components/lifecycle
- 2: https://v17.angular.io/api/core/AfterContentInit
- 3: https://v17.angular.io/guide/lifecycle-hooks
- 4: https://v2.angular.io/docs/ts/latest/guide/lifecycle-hooks.html
- 5: https://angular.dev/api/core/ContentChildren
- 6: https://v17.angular.io/api/core/ContentChildren
- 7: https://medium.com/@jamesbenrobb/angular-input-and-contentchildren-signal-gotcha-dont-use-an-effect-to-initialise-the-child-object-3af19a4d822c
- 8: https://briantree.se/understanding-view-vs-content-in-angular/
Do not update dropdown.open from ngAfterContentInit.
ngAfterContentInit runs by the time the projected dropdown content is initialized, but after Angular has already checked this component’s bindings. Setting dropdown.open here can trigger ExpressionChangedAfterItHasBeenCheckedError in dev mode when defaultOpen() is true. Move the initialization to earlier controlled state/deep props before rendering, or defer it to afterNextRender/a next tick after the hook.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tedi/components/layout/sidenav/sidenav-item/sidenav-item.component.ts` around
lines 102 - 108, Remove the dropdown.open update from ngAfterContentInit;
initialize the expanded state through earlier controlled state/deep props before
rendering, or defer the update with afterNextRender/next-tick scheduling so it
does not change a checked binding during the hook.
1 participant
BREAKING CHANGE: The
<tedi-sidenav-*>element API is removed. Wrap items in<ul tedi-sidenav-list>and use the attribute forms instead:<tedi-sidenav-item>-><li tedi-sidenav-item>(same fordropdown,dropdown-item,group-title). Dropdown groups now use[tedi-sidenav-dropdown-group-parent]+<ul tedi-sidenav-dropdown-group-list>.https://storybook.tedi.ee/angular/feat/307-sidenav-component-api-restructuring/?path=/docs/tedi-ready-layout-sidenav--docs
Summary by CodeRabbit
Summary
New Features
<ul>/<li>markup with improved navigation structure.defaultCollapsed,defaultOpen,collapsedText, andariaLabeloptions.Bug Fixes
Documentation
Tests