fix(sidebar): update state when responsive setting change (#2654)

Author: yggg

src/framework/theme/components/sidebar/sidebar.component.ts

@@ -16,8 +16,8 @@ import {
   OnInit,
   Output,
 } from '@angular/core';
-import { Subject } from 'rxjs';
-import { takeUntil, filter } from 'rxjs/operators';
+import { combineLatest, Subject } from 'rxjs';
+import { takeUntil, filter, map, startWith } from 'rxjs/operators';
 
 import { convertToBoolProperty, NbBooleanInput } from '../helpers';
 import { NbThemeService } from '../../services/theme.service';
@@ -146,6 +146,7 @@ export class NbSidebarFooterComponent {
 })
 export class NbSidebarComponent implements OnInit, OnDestroy {
 
+  protected readonly responsiveValueChange$: Subject<boolean> = new Subject<boolean>();
   protected responsiveState: NbSidebarResponsiveState = 'pc';
 
   protected destroy$ = new Subject<void>();
@@ -254,7 +255,7 @@ export class NbSidebarComponent implements OnInit, OnDestroy {
   set state(value: NbSidebarState) {
     this._state = value;
   }
-  protected _state: NbSidebarState;
+  protected _state: NbSidebarState = 'expanded';
 
   /**
    * Makes sidebar listen to media query events and change its behaviour
@@ -265,7 +266,10 @@ export class NbSidebarComponent implements OnInit, OnDestroy {
     return this._responsive;
   }
   set responsive(value: boolean) {
-    this._responsive = convertToBoolProperty(value);
+    if (this.responsive !== convertToBoolProperty(value)) {
+      this._responsive = !this.responsive;
+      this.responsiveValueChange$.next(this.responsive);
+    }
   }
   protected _responsive: boolean = false;
   static ngAcceptInputType_responsive: NbBooleanInput;
@@ -344,13 +348,26 @@ export class NbSidebarComponent implements OnInit, OnDestroy {
       .subscribe(() => this.compact());
 
     getSidebarState$
-      .pipe(filter(({ tag }) => !this.tag || this.tag === tag))
+      .pipe(
+        filter(({ tag }) => !this.tag || this.tag === tag),
+        takeUntil(this.destroy$),
+      )
       .subscribe(({ observer }) => observer.next(this.state));
 
     getSidebarResponsiveState$
-      .pipe(filter(({ tag }) => !this.tag || this.tag === tag))
+      .pipe(
+        filter(({ tag }) => !this.tag || this.tag === tag),
+        takeUntil(this.destroy$),
+      )
       .subscribe(({ observer }) => observer.next(this.responsiveState));
 
+    this.responsiveValueChange$
+      .pipe(
+        filter((responsive: boolean) => !responsive),
+        takeUntil(this.destroy$),
+      )
+      .subscribe(() => this.expand());
+
     this.subscribeToMediaQueryChange();
   }
 
@@ -376,27 +393,21 @@ export class NbSidebarComponent implements OnInit, OnDestroy {
    * Collapses the sidebar
    */
   collapse() {
-    this.state = 'collapsed';
-    this.stateChange.emit(this.state);
-    this.cd.markForCheck();
+    this.updateState('collapsed');
   }
 
   /**
    * Expands the sidebar
    */
   expand() {
-    this.state = 'expanded';
-    this.stateChange.emit(this.state);
-    this.cd.markForCheck();
+    this.updateState('expanded');
   }
 
   /**
    * Compacts the sidebar (minimizes)
    */
   compact() {
-    this.state = 'compacted';
-    this.stateChange.emit(this.state);
-    this.cd.markForCheck();
+    this.updateState('compacted');
   }
 
   /**
@@ -418,18 +429,20 @@ export class NbSidebarComponent implements OnInit, OnDestroy {
     }
 
     if (this.state === 'compacted' || this.state === 'collapsed') {
-      this.state = 'expanded';
+      this.updateState('expanded');
     } else {
-      this.state = compact ? 'compacted' : 'collapsed';
+      this.updateState(compact ? 'compacted' : 'collapsed');
     }
-    this.stateChange.emit(this.state);
-    this.cd.markForCheck();
   }
 
   protected subscribeToMediaQueryChange() {
-    this.themeService.onMediaQueryChange()
+    combineLatest([
+      this.responsiveValueChange$.pipe(startWith(this.responsive)),
+      this.themeService.onMediaQueryChange(),
+    ])
       .pipe(
-        filter(() => this.responsive),
+        filter(([responsive]) => responsive),
+        map(([, breakpoints]) => breakpoints),
         takeUntil(this.destroy$),
       )
       .subscribe(([prev, current]: [NbMediaBreakpoint, NbMediaBreakpoint]) => {
@@ -449,7 +462,7 @@ export class NbSidebarComponent implements OnInit, OnDestroy {
           this.collapse();
           newResponsiveState = 'mobile';
         }
-        if (!isCollapsed && !isCompacted && prev.width < current.width) {
+        if (!isCollapsed && !isCompacted && (!prev.width || prev.width < current.width)) {
           this.expand();
           this.fixed = false;
           newResponsiveState = 'pc';
@@ -475,6 +488,14 @@ export class NbSidebarComponent implements OnInit, OnDestroy {
     return this.getMenuLink(element.parentElement);
   }
 
+  protected updateState(state: NbSidebarState): void {
+    if (this.state !== state) {
+      this.state = state;
+      this.stateChange.emit(this.state);
+      this.cd.markForCheck();
+    }
+  }
+
   /**
    * @deprecated Use `responsive` property instead
    * @breaking-change Remove @8.0.0

src/framework/theme/components/sidebar/sidebar.spec.ts

@@ -1,8 +1,10 @@
-import { Component, DebugElement } from '@angular/core';
+import { Component, DebugElement, Injectable } from '@angular/core';
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { By } from '@angular/platform-browser';
 import { RouterTestingModule } from '@angular/router/testing';
 import { NoopAnimationsModule } from '@angular/platform-browser/animations';
+import { Observable, Subject } from 'rxjs';
+import { pairwise, startWith } from 'rxjs/operators';
 import {
   NbSidebarComponent,
   NbMenuModule,
@@ -11,12 +13,17 @@ import {
   NbMenuComponent,
   NbThemeModule,
   NbMenuItemComponent,
-  NbIconComponent, NbSidebarService,
+  NbIconComponent,
+  NbSidebarService,
+  NbSidebarState,
+  NbMediaBreakpoint,
+  NbThemeService,
+  NbMediaBreakpointsService,
 } from '@nebular/theme';
 
 @Component({
   template: `
-    <nb-sidebar>
+    <nb-sidebar [responsive]="responsive" [state]="state">
       <button id="button-outside-menu"></button>
       <nb-menu [items]="menuItems"></nb-menu>
     </nb-sidebar>
@@ -36,6 +43,32 @@ export class SidebarExpandTestComponent {
       group: true,
     },
   ];
+
+  responsive = false;
+  state: NbSidebarState = 'expanded';
+}
+
+@Injectable()
+export class MockThemeService {
+  private breakpoint$ = new Subject<NbMediaBreakpoint>();
+
+  constructor(private breakpointsService: NbMediaBreakpointsService) {
+  }
+
+  setBreakpointTo(breakpointName: string): void {
+    this.breakpoint$.next(this.breakpointsService.getByName(breakpointName));
+  }
+
+  onMediaQueryChange(): Observable<NbMediaBreakpoint[]> {
+    const breakpoints = this.breakpointsService.getBreakpoints();
+    const largestBreakpoint = breakpoints[breakpoints.length - 1];
+
+    return this.breakpoint$
+      .pipe(
+        startWith({ name: 'unknown', width: undefined }, largestBreakpoint),
+        pairwise(),
+      );
+  }
 }
 
 describe('NbSidebarComponent', () => {
@@ -48,19 +81,25 @@ describe('NbSidebarComponent', () => {
         NbSidebarModule.forRoot(),
         NbMenuModule.forRoot(),
       ],
+      providers: [
+        MockThemeService,
+        { provide: NbThemeService, useExisting: MockThemeService },
+      ],
       declarations: [ SidebarExpandTestComponent ],
     });
   });
 
   describe('States (expanded, collapsed, compacted)', () => {
     let fixture: ComponentFixture<SidebarExpandTestComponent>;
     let sidebarComponent: NbSidebarComponent;
+    let themeService: MockThemeService;
 
     beforeEach(() => {
       fixture = TestBed.createComponent(SidebarExpandTestComponent);
       fixture.detectChanges();
 
       sidebarComponent = fixture.debugElement.query(By.directive(NbSidebarComponent)).componentInstance;
+      themeService = TestBed.inject(MockThemeService);
     });
 
     it(`should collapse when collapse method called`, () => {
@@ -142,5 +181,48 @@ describe('NbSidebarComponent', () => {
 
       expect(sidebarComponent.compacted).toEqual(true);
     });
+
+    it('should expand when responsive is set to false', () => {
+      sidebarComponent.responsive = true;
+      sidebarComponent.collapse();
+
+      expect(sidebarComponent.state).toEqual('collapsed');
+
+      sidebarComponent.responsive = false;
+
+      expect(sidebarComponent.state).toEqual('expanded');
+    });
+
+    it('should update state according to the current breakpoint when responsive turns on (update to compacted)', () => {
+      const compactedBreakpoints = sidebarComponent.compactedBreakpoints;
+      const largestCompactedBreakpointName = compactedBreakpoints[compactedBreakpoints.length - 1];
+      themeService.setBreakpointTo(largestCompactedBreakpointName);
+
+      expect(sidebarComponent.state).toEqual('expanded');
+
+      sidebarComponent.responsive = true;
+
+      expect(sidebarComponent.state).toEqual('compacted');
+    });
+
+    it('should update state according to the current breakpoint when responsive turns on (update to collapsed)', () => {
+      themeService.setBreakpointTo(sidebarComponent.collapsedBreakpoints[0]);
+
+      expect(sidebarComponent.state).toEqual('expanded');
+
+      sidebarComponent.responsive = true;
+
+      expect(sidebarComponent.state).toEqual('collapsed');
+    });
+
+    it('should expand when responsive and initial state is in different breakpoint', () => {
+      fixture = TestBed.createComponent(SidebarExpandTestComponent);
+      fixture.componentInstance.responsive = true;
+      fixture.componentInstance.state = 'compacted';
+      fixture.detectChanges();
+      sidebarComponent = fixture.debugElement.query(By.directive(NbSidebarComponent)).componentInstance;
+
+      expect(sidebarComponent.state).toEqual('expanded');
+    });
   });
 });