From 9ca191ef405d89f6be5c5d6a8e3c4a43bf3a8666 Mon Sep 17 00:00:00 2001 From: xidedix Date: Fri, 16 Oct 2026 16:15:49 +0200 Subject: [PATCH] fix(Sidebar): apply input changes after first render, scope state actions to their sidebar - ngOnChanges matched alias keys; SimpleChanges carries visibleInput/narrowInput/unfoldableInput, so [visible] left hide + inert. Keys typed via SimpleChanges; ngOnInit flag replaces firstChange, first setInput() after render applies. - Actions with a sidebar reference (init, layout, backdrop, mobile nav link) reached every id-less sidebar; now only that sidebar. Actions without one still match by id. - Action without visible keeps visibility; sidebar clears the shared backdrop only when it was the open one. --- .../sidebar/sidebar/sidebar.component.spec.ts | 188 +++++++++++++++++- .../lib/sidebar/sidebar/sidebar.component.ts | 51 ++--- 2 files changed, 212 insertions(+), 27 deletions(-) diff --git a/projects/coreui-angular/src/lib/sidebar/sidebar/sidebar.component.spec.ts b/projects/coreui-angular/src/lib/sidebar/sidebar/sidebar.component.spec.ts index ebbebfa2..0706b84d 100644 --- a/projects/coreui-angular/src/lib/sidebar/sidebar/sidebar.component.spec.ts +++ b/projects/coreui-angular/src/lib/sidebar/sidebar/sidebar.component.spec.ts @@ -1,5 +1,5 @@ import { BreakpointObserver } from '@angular/cdk/layout'; -import { Component } from '@angular/core'; +import { Component, signal } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { By } from '@angular/platform-browser'; import { provideRouter } from '@angular/router'; @@ -7,7 +7,7 @@ import { Subject } from 'rxjs'; import { map } from 'rxjs/operators'; import { SidebarComponent } from './sidebar.component'; -import { SidebarService } from '../sidebar.service'; +import { ISidebarAction, SidebarService } from '../sidebar.service'; import { SidebarNavComponent } from '../sidebar-nav/sidebar-nav.component'; describe('SidebarComponent', () => { @@ -45,6 +45,13 @@ describe('SidebarComponent', () => { expect(fixture.nativeElement.inert).toBe(true); }); + it('should apply the first input set after the first render', async () => { + fixture.componentRef.setInput('visible', true); + await fixture.whenStable(); + expect(fixture.nativeElement.classList.contains('show')).toBe(true); + expect(component.sidebarState.visible).toBe(true); + }); + it('should clear the backdrop when a visible mobile sidebar is destroyed', async () => { component.sidebarState = { mobile: true, visible: true }; await fixture.whenStable(); @@ -112,6 +119,13 @@ describe('SidebarComponent state', () => { expect(startElement.classList.contains('hide')).toBe(true); }); + it('should apply a state with a sidebar reference only to that sidebar', async () => { + service.toggle({ toggle: 'visible', sidebar: start, id: 'end' }); + await fixture.whenStable(); + expect(startElement.classList.contains('show')).toBe(true); + expect(endElement.classList.contains('hide')).toBe(true); + }); + it('should narrow through the narrow setter unless unfoldable', async () => { start.narrow = true; await fixture.whenStable(); @@ -125,6 +139,152 @@ describe('SidebarComponent state', () => { }); }); +@Component({ + template: ` + + `, + imports: [SidebarComponent] +}) +class BoundSidebarComponent { + readonly narrow = signal(false); + readonly overlaid = signal(false); + readonly unfoldable = signal(false); + readonly visible = signal(false); +} + +describe('SidebarComponent inputs', () => { + let fixture: ComponentFixture; + let host: BoundSidebarComponent; + let sidebar: SidebarComponent; + let element: HTMLElement; + + beforeEach(async () => { + fixture = TestBed.createComponent(BoundSidebarComponent); + await fixture.whenStable(); + host = fixture.componentInstance; + const sidebarRef = fixture.debugElement.query(By.directive(SidebarComponent)); + sidebar = sidebarRef.componentInstance; + element = sidebarRef.nativeElement; + }); + + it('should send only its initial state to the sidebar service on init', async () => { + const actions: ISidebarAction[] = []; + TestBed.inject(SidebarService).sidebarState$.subscribe((action) => actions.push(action)); + actions.length = 0; + + const bound = TestBed.createComponent(BoundSidebarComponent); + await bound.whenStable(); + expect(actions.length).toBe(1); + }); + + it('should show and hide when the visible input changes', async () => { + host.visible.set(true); + await fixture.whenStable(); + expect(element.classList.contains('show')).toBe(true); + expect(element.inert).toBeFalsy(); + expect(sidebar.sidebarState.visible).toBe(true); + + host.visible.set(false); + await fixture.whenStable(); + expect(element.classList.contains('hide')).toBe(true); + expect(element.inert).toBe(true); + expect(sidebar.sidebarState.visible).toBe(false); + }); + + it('should apply a narrow input change to the sidebar state', async () => { + host.narrow.set(true); + await fixture.whenStable(); + expect(element.classList.contains('sidebar-narrow')).toBe(true); + expect(sidebar.sidebarState.narrow).toBe(true); + }); + + it('should toggle unfoldable off after the unfoldable input turns it on', async () => { + host.unfoldable.set(true); + await fixture.whenStable(); + expect(element.classList.contains('sidebar-narrow-unfoldable')).toBe(true); + + TestBed.inject(SidebarService).toggle({ toggle: 'unfoldable', sidebar }); + await fixture.whenStable(); + expect(element.classList.contains('sidebar-narrow-unfoldable')).toBe(false); + }); + + it('should update a two-way visible binding on toggle', async () => { + const service = TestBed.inject(SidebarService); + + service.toggle({ toggle: 'visible', sidebar }); + await fixture.whenStable(); + expect(host.visible()).toBe(true); + expect(element.classList.contains('show')).toBe(true); + + service.toggle({ toggle: 'visible', sidebar }); + await fixture.whenStable(); + expect(host.visible()).toBe(false); + expect(element.classList.contains('hide')).toBe(true); + }); + + it('should keep an open overlaid sidebar open when the narrow input changes', async () => { + host.overlaid.set(true); + await fixture.whenStable(); + TestBed.inject(SidebarService).toggle({ toggle: 'visible', sidebar }); + await fixture.whenStable(); + + host.narrow.set(true); + await fixture.whenStable(); + expect(element.classList.contains('show')).toBe(true); + expect(host.visible()).toBe(true); + }); +}); + +@Component({ + template: ` + + + + `, + imports: [SidebarComponent] +}) +class SidebarsWithoutIdComponent {} + +describe('SidebarComponent without id', () => { + let fixture: ComponentFixture; + let sidebars: SidebarComponent[]; + let elements: HTMLElement[]; + + beforeEach(async () => { + fixture = TestBed.createComponent(SidebarsWithoutIdComponent); + await fixture.whenStable(); + const refs = fixture.debugElement.queryAll(By.directive(SidebarComponent)); + sidebars = refs.map((ref) => ref.componentInstance); + elements = refs.map((ref) => ref.nativeElement); + }); + + it('should keep its own initial state', () => { + expect(elements[1].classList.contains('show')).toBe(true); + expect(sidebars[1].sidebarState.sidebar).toBe(sidebars[1]); + }); + + it('should apply a state addressed to another sidebar only to that sidebar', async () => { + TestBed.inject(SidebarService).toggle({ toggle: 'visible', sidebar: sidebars[2] }); + await fixture.whenStable(); + expect(elements[2].classList.contains('show')).toBe(true); + expect(elements[1].classList.contains('show')).toBe(true); + expect(elements[0].classList.contains('hide')).toBe(true); + expect(sidebars[1].sidebarState.sidebar).toBe(sidebars[1]); + + TestBed.inject(SidebarService).toggle({ toggle: 'visible', sidebar: sidebars[2] }); + await fixture.whenStable(); + expect(elements[2].classList.contains('hide')).toBe(true); + expect(elements[1].classList.contains('show')).toBe(true); + }); + + it('should apply a state without a sidebar reference to the sidebars matching its id', async () => { + TestBed.inject(SidebarService).toggle({ visible: true }); + await fixture.whenStable(); + expect(elements[2].classList.contains('show')).toBe(true); + expect(elements[0].classList.contains('hide')).toBe(true); + }); +}); + @Component({ template: ` @@ -137,6 +297,17 @@ class MobileSidebarComponent { readonly navItems = [{ name: 'Dashboard', url: '/dashboard' }]; } +@Component({ + template: ` + + + `, + imports: [SidebarComponent] +}) +class MobileSidebarPairComponent { + readonly unfoldable = signal(false); +} + describe('SidebarComponent on mobile', () => { let fixture: ComponentFixture; let layout: Subject; @@ -214,6 +385,19 @@ describe('SidebarComponent on mobile', () => { expect(document.querySelector('.sidebar-backdrop')).toBeNull(); }); + it('should keep the backdrop of an open sidebar when a closed sidebar changes', async () => { + const pair = TestBed.createComponent(MobileSidebarPairComponent); + await pair.whenStable(); + await setMobile(true); + TestBed.inject(SidebarService).toggle({ toggle: 'visible', id: 'main' }); + await pair.whenStable(); + expect(document.querySelector('.sidebar-backdrop')).not.toBeNull(); + + pair.componentInstance.unfoldable.set(true); + await pair.whenStable(); + expect(document.querySelector('.sidebar-backdrop')).not.toBeNull(); + }); + it('should hide after a nav link click on mobile only', async () => { const link = (): HTMLElement => fixture.nativeElement.querySelector('c-sidebar-nav a'); diff --git a/projects/coreui-angular/src/lib/sidebar/sidebar/sidebar.component.ts b/projects/coreui-angular/src/lib/sidebar/sidebar/sidebar.component.ts index 6e135c28..3b5d485f 100644 --- a/projects/coreui-angular/src/lib/sidebar/sidebar/sidebar.component.ts +++ b/projects/coreui-angular/src/lib/sidebar/sidebar/sidebar.component.ts @@ -38,6 +38,7 @@ export class SidebarComponent implements OnChanges, OnDestroy, OnInit { readonly #sidebarService = inject(SidebarService); readonly #backdropService = inject(SidebarBackdropService); + #initialized = false; #onMobile = false; #layoutChangeSubscription!: Subscription; #stateToggleSubscription!: Subscription; @@ -148,6 +149,7 @@ export class SidebarComponent implements OnChanges, OnDestroy, OnInit { set sidebarState(value: ISidebarAction) { const newState = value; + const wasOpen = this.state().mobile && this.state().visible; if ('toggle' in newState) { if (newState.toggle === 'visible') { newState.visible = !this.state().visible; @@ -156,13 +158,15 @@ export class SidebarComponent implements OnChanges, OnDestroy, OnInit { newState.unfoldable = !this.state().unfoldable; this.unfoldable.set(newState.unfoldable); } - } else { - this.#visible.update((visible) => (newState.visible ?? visible) && !this.overlaid()); + } else if (newState.visible !== undefined) { + this.#visible.set(newState.visible && !this.overlaid()); } this.state.update((state) => ({ ...state, ...newState })); - this.state().mobile && this.state().visible - ? this.#backdropService.setBackdrop(this) - : this.#backdropService.clearBackdrop(); + if (this.state().mobile && this.state().visible) { + this.#backdropService.setBackdrop(this); + } else if (wasOpen) { + this.#backdropService.clearBackdrop(); + } } get sidebarState(): ISidebarAction { @@ -209,6 +213,7 @@ export class SidebarComponent implements OnChanges, OnDestroy, OnInit { this.setInitialState(); this.layoutChangeSubscribe(); this.stateToggleSubscribe(); + this.#initialized = true; } ngOnDestroy(): void { @@ -219,27 +224,23 @@ export class SidebarComponent implements OnChanges, OnDestroy, OnInit { } } - ngOnChanges(changes: SimpleChanges): void { - const oldStateMap = new Map(Object.entries(this.state())); - const newStateMap = new Map(); - newStateMap.set('sidebar', this); - - const propList = ['visible', 'unfoldable', 'narrow']; - - for (const propName in changes) { - if (propList.includes(propName)) { - if (changes[propName] && !changes[propName].firstChange) { - const value = booleanAttribute(changes[propName].currentValue); - if (oldStateMap.get(propName) !== value) { - newStateMap.set(propName, value); - } - } + ngOnChanges(changes: SimpleChanges): void { + if (!this.#initialized) { + return; + } + const state = this.state(); + const newState: ISidebarAction = {}; + for (const [key, change] of [ + ['narrow', changes.narrowInput], + ['unfoldable', changes.unfoldableInput], + ['visible', changes.visibleInput] + ] as const) { + if (change && state[key] !== change.currentValue) { + newState[key] = change.currentValue; } } - - if (newStateMap.size > 1) { - const state = Object.fromEntries(newStateMap.entries()); - this.#sidebarService.toggle(state); + if (Object.keys(newState).length) { + this.#sidebarService.toggle({ ...newState, sidebar: this }); } } @@ -258,7 +259,7 @@ export class SidebarComponent implements OnChanges, OnDestroy, OnInit { private stateToggleSubscribe(subscribe: boolean = true): void { if (subscribe) { this.#stateToggleSubscription = this.#sidebarService.sidebarState$.subscribe((state) => { - if (this === state.sidebar || this.id() === state.id) { + if (state.sidebar ? this === state.sidebar : this.id() === state.id) { this.sidebarState = { ...state }; } });