Skip to content

Navbar: responsive auto-collapse (expand breakpoint) never activates #269

Description

@mrholek

Summary

The Navbar's responsive auto-collapse on breakpoint (expand="sm|md|lg|xl|xxl") never activates — the BreakpointObserver subscription that toggles the inner CollapseDirective is set up inside ngAfterContentInit() guarded by if (this.breakpoint()), but at that lifecycle point breakpoint() is still empty, so the block is skipped and the documented feature is silently dead. Affects free and pro (projects/coreui-angular/src/lib/navbar/navbar.component.ts:105, identical in both).

Root cause

readonly computedStyle = signal<string>('');

readonly #afterEveryRenderFn = afterEveryRender({
  read: () => {
    // reads --cui-breakpoint-<expand> from computed styles and sets computedStyle
    computedStyle && this.computedStyle.set(computedStyle);
  }
});

readonly breakpoint = computed(() => {
  const expand = this.expand();
  if (typeof expand === 'string') {
    return this.computedStyle();   // '' until afterEveryRender runs
  }
  return false;
});

ngAfterContentInit(): void {
  const breakpoint = this.breakpoint();   // '' here → falsy
  if (breakpoint) {                        // never entered
    this.#observer = this.#breakpointObserver.observe([`(min-width: ${breakpoint})`])...
  }
}

breakpoint() is derived (via computed) from the computedStyle signal, which is only populated by the afterEveryRender read hook. That hook runs after ngAfterContentInit(), so when ngAfterContentInit() reads this.breakpoint() it gets '' (falsy), the if is skipped, and the observer is never created. ngAfterContentInit is one-shot, so it never re-reads the value once computedStyle fills in. Net result: setting expand renders the correct navbar-expand-* CSS class, but the JS-driven collapse toggling on viewport changes never runs.

Suggested fix

Replace the one-shot ngAfterContentInit with an effect() that reacts to breakpoint() (and collapse()), so it wires up the observer as soon as computedStyle resolves — and re-wires if expand changes. Use the effect's onCleanup to unsubscribe the previous observer, which also removes the manual ngOnDestroy/#observer bookkeeping.

readonly #breakpointEffect = effect((onCleanup) => {
  const breakpoint = this.breakpoint();
  if (!breakpoint) {
    return;
  }

  const subscription = this.#breakpointObserver
    .observe([`(min-width: ${breakpoint})`])
    .subscribe((result) => {
      const collapse = this.collapse();
      if (!collapse) {
        return;
      }
      const animate = collapse.animate();
      collapse.animate.set(false);
      collapse.toggle(false);
      setTimeout(() => {
        collapse.toggle(result.matches);
        setTimeout(() => collapse.animate.set(animate));
      });
    });

  onCleanup(() => subscription.unsubscribe());
});

Notes:

  • effect runs after the first render and re-runs whenever breakpoint() changes, so it picks up the value afterEveryRender writes into computedStyle — fixing the ordering problem that ngAfterContentInit can't.
  • onCleanup replaces the manual #observer + ngOnDestroy().unsubscribe().
  • Keep the existing afterEveryRender read hook that populates computedStyle.

How to verify

A component test that mounts <c-navbar expand="md"><... c-collapse></c-navbar>, stubs BreakpointObserver.observe to emit { matches: false } / { matches: true }, and asserts the inner CollapseDirective.toggle is called accordingly. Before the fix the observer is never subscribed (toggle never called); after, it toggles on breakpoint changes.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions