Fix: Close advanced filter slideout when selecting an item (#1846)

Author: timotheeguerin

src/@batch-flask/ui/browse-layout/browse-layout.component.spec.ts

@@ -0,0 +1,103 @@
+import { Component, DebugElement, Injector, forwardRef } from "@angular/core";
+import { ComponentFixture, TestBed } from "@angular/core/testing";
+import { FormsModule, ReactiveFormsModule } from "@angular/forms";
+import { By } from "@angular/platform-browser";
+import { RouterTestingModule } from "@angular/router/testing";
+import { I18nTestingModule } from "@batch-flask/core/testing";
+import { click } from "test/utils/helpers";
+import { ListBaseComponent } from "../abstract-list";
+import { ButtonsModule } from "../buttons";
+import { ScrollableModule } from "../scrollable";
+import { SplitPaneModule } from "../split-pane";
+import { BrowseLayoutAdvancedFilterDirective } from "./browse-layout-advanced-filter";
+import { BrowseLayoutListDirective } from "./browse-layout-list";
+import { BrowseLayoutComponent } from "./browse-layout.component";
+import { ToggleFilterButtonComponent } from "./toggle-filter-button";
+
+@Component({
+    selector: "bl-fake-list",
+    template: `
+    `,
+    providers: [{
+        provide: ListBaseComponent,
+        useExisting: forwardRef(() => FakeListComponent),
+    }],
+})
+class FakeListComponent extends ListBaseComponent {
+    constructor(injector: Injector) {
+        super(injector);
+    }
+}
+
+@Component({
+    template: `
+        <bl-browse-layout>
+            <div blBrowseLayoutTitle>
+                Jobs
+            </div>
+            <div blBrowseLayoutButtons>
+            </div>
+            <bl-fake-list blBrowseLayoutList></bl-fake-list>
+            <div blBrowseLayoutAdvancedFilter >
+                Advanced filter contents
+            </div>
+        </bl-browse-layout>
+    `,
+})
+class TestComponent {
+}
+
+describe("BrowseLayoutComponent", () => {
+    let fixture: ComponentFixture<TestComponent>;
+    let fakeList: FakeListComponent;
+    let de: DebugElement;
+
+    let advFilterEl: DebugElement;
+
+    beforeEach(() => {
+        TestBed.configureTestingModule({
+            imports: [
+                ScrollableModule,
+                ButtonsModule,
+                SplitPaneModule,
+                FormsModule,
+                ReactiveFormsModule,
+                RouterTestingModule,
+                I18nTestingModule,
+            ],
+            declarations: [
+                BrowseLayoutComponent,
+                TestComponent,
+                FakeListComponent,
+                BrowseLayoutComponent,
+                BrowseLayoutListDirective,
+                ToggleFilterButtonComponent,
+                BrowseLayoutAdvancedFilterDirective,
+            ],
+        });
+        fixture = TestBed.createComponent(TestComponent);
+        de = fixture.debugElement.query(By.css("bl-browse-layout"));
+        fixture.detectChanges();
+
+        fakeList = de.query(By.directive(FakeListComponent)).componentInstance;
+        advFilterEl = de.query(By.css(".advanced-filter-content"));
+    });
+
+    describe("When the advanced filter is opened", () => {
+        beforeEach(async () => {
+            click(de.query(By.css("bl-toggle-filter-button bl-clickable")));
+            fixture.detectChanges();
+            await fixture.whenStable();
+        });
+
+        it("has the advanced filter open", () => {
+            expect(advFilterEl).toBeVisible();
+        });
+
+        it("close the advanced filters when selecting an item", () => {
+            fakeList.activeItemChange.emit("foo");
+            fixture.detectChanges();
+            expect(advFilterEl).not.toBeVisible();
+        });
+    });
+});

src/@batch-flask/ui/browse-layout/browse-layout.component.ts

@@ -1,21 +1,21 @@
+import { LiveAnnouncer } from "@angular/cdk/a11y";
 import {
-    AfterContentInit, ChangeDetectionStrategy,
-    ChangeDetectorRef, Component, ContentChild,
-    ElementRef, Input, OnChanges, OnInit, ViewChild,
+    AfterContentInit, AfterViewInit,
+    ChangeDetectionStrategy, ChangeDetectorRef, Component,
+    ContentChild, ElementRef, Input, OnChanges, OnDestroy, OnInit, ViewChild,
 } from "@angular/core";
 import { FormControl } from "@angular/forms";
 import { ActivatedRoute } from "@angular/router";
 import { Filter, FilterBuilder, I18nService, autobind } from "@batch-flask/core";
 import { KeyCode } from "@batch-flask/core/keys";
 import { ListSelection } from "@batch-flask/core/list";
 import { SanitizedError } from "@batch-flask/utils";
-import { Subscription } from "rxjs";
-import { debounceTime, distinctUntilChanged } from "rxjs/operators";
+import { Subject } from "rxjs";
+import { debounceTime, distinctUntilChanged, takeUntil } from "rxjs/operators";
 import { SplitPaneComponent, SplitPaneConfig } from "../split-pane";
 import { BrowseLayoutAdvancedFilterDirective } from "./browse-layout-advanced-filter";
 import { BrowseLayoutListDirective } from "./browse-layout-list";
 
-import { LiveAnnouncer } from "@angular/cdk/a11y";
 import "./browse-layout.scss";
 
 export interface BrowseLayoutConfig {
@@ -46,7 +46,7 @@ let idCounter = 0;
     templateUrl: "browse-layout.html",
     changeDetection: ChangeDetectionStrategy.OnPush,
 })
-export class BrowseLayoutComponent implements OnInit, AfterContentInit, OnChanges {
+export class BrowseLayoutComponent implements OnInit, AfterViewInit, AfterContentInit, OnChanges, OnDestroy {
     @Input() public id = `bl-browse-layout-${idCounter++}`;
     /**
      * Field for the quicksearch.
@@ -94,20 +94,14 @@ export class BrowseLayoutComponent implements OnInit, AfterContentInit, OnChange
 
     private _activeItemKey: string = null;
     private _config: BrowseLayoutConfig = defaultConfig;
-    private _selectionChangeSub: Subscription;
+    private _destroy = new Subject();
 
     constructor(
-        activeRoute: ActivatedRoute,
+        private activeRoute: ActivatedRoute,
         private i18n: I18nService,
         private liveAnouncer: LiveAnnouncer,
         private changeDetector: ChangeDetectorRef) {
 
-        activeRoute.queryParams.subscribe((params: any) => {
-            if (params.filter) {
-                this.toggleFilter(true);
-            }
-        });
-
         activeRoute.url.subscribe((url) => {
             const child = activeRoute.snapshot.firstChild;
             if (child) {
@@ -136,6 +130,13 @@ export class BrowseLayoutComponent implements OnInit, AfterContentInit, OnChange
         });
     }
 
+    public ngAfterViewInit() {
+        this.activeRoute.queryParams.pipe(takeUntil(this._destroy)).subscribe((params: any) => {
+            if (params.filter) {
+                this.toggleFilter(true);
+            }
+        });
+    }
     public ngOnChanges(changes) {
         if (changes.config) {
             this._updateFilter();
@@ -153,16 +154,21 @@ export class BrowseLayoutComponent implements OnInit, AfterContentInit, OnChange
         this.deleteSelectionIsEnabled = Boolean(component.deleteSelection);
         this.refreshEnabled = Boolean(component.refresh);
         this.changeDetector.markForCheck();
-        this._selectionChangeSub = this.listDirective.component.selectionChange.subscribe((x) => {
+        this.listDirective.component.selectionChange.pipe(takeUntil(this._destroy)).subscribe((x) => {
             this.selection = x;
             this.changeDetector.markForCheck();
         });
+
+        this.listDirective.component.activeItemChange.pipe(takeUntil(this._destroy)).subscribe((x) => {
+            if (this.showAdvancedFilter) {
+                this.toggleFilter(false);
+            }
+        });
     }
 
-    public _ngOnDestroy() {
-        if (this._selectionChangeSub) {
-            this._selectionChangeSub.unsubscribe();
-        }
+    public ngOnDestroy() {
+        this._destroy.next();
+        this._destroy.complete();
     }
 
     /**
@@ -181,7 +187,7 @@ export class BrowseLayoutComponent implements OnInit, AfterContentInit, OnChange
 
     public toggleFilter(value?: boolean) {
         const newValue = (value === undefined ? !this.showAdvancedFilter : value);
-        if (newValue === this.showAdvancedFilter) {return; }
+        if (newValue === this.showAdvancedFilter) { return; }
         this.showAdvancedFilter = newValue;
 
         if (this.listDirective) {

src/@batch-flask/ui/browse-layout/browse-layout.module.ts

@@ -17,7 +17,8 @@ const publicComponents = [
     BrowseLayoutComponent,
     BrowseLayoutListDirective,
     ToggleFilterButtonComponent,
-    BrowseLayoutAdvancedFilterDirective];
+    BrowseLayoutAdvancedFilterDirective,
+];
 
 @NgModule({
     imports: [