Skip to content

Commit

Permalink
fix(expansion): respect parent accordion hideToggle binding
Browse files Browse the repository at this point in the history
* Fixes that the expansion panels no longer respect the bindings of a parent `<mat-accordion>`. (this has been accidentally changes in angular#6529)
* Fixes that the expansion panel does not re-render if the `[hideToggle]` binding is set on the accordion.
* Removes the unused `_getHideToggle` method. This method is no longer used because the panel header directly accesses the panel through DI.
  • Loading branch information
devversion committed Aug 18, 2018
1 parent b9651df commit 616cb57
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 21 deletions.
15 changes: 13 additions & 2 deletions src/cdk/accordion/accordion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Directive, Input} from '@angular/core';
import {coerceBooleanProperty} from '@angular/cdk/coercion';
import {Directive, Input, OnChanges, OnDestroy, SimpleChanges} from '@angular/core';
import {Subject} from 'rxjs';

/** Used to generate unique ID for each accordion. */
Expand All @@ -20,7 +20,10 @@ let nextId = 0;
selector: 'cdk-accordion, [cdkAccordion]',
exportAs: 'cdkAccordion',
})
export class CdkAccordion {
export class CdkAccordion implements OnDestroy, OnChanges {
/** Emits when the state of the accordion changes */
readonly _stateChanges = new Subject<SimpleChanges>();

/** Stream that emits true/false when openAll/closeAll is triggered. */
readonly _openCloseAllActions: Subject<boolean> = new Subject<boolean>();

Expand All @@ -43,6 +46,14 @@ export class CdkAccordion {
this._openCloseAll(false);
}

ngOnChanges(changes: SimpleChanges) {
this._stateChanges.next(changes);
}

ngOnDestroy() {
this._stateChanges.complete();
}

private _openCloseAll(expanded: boolean): void {
if (this.multi) {
this._openCloseAllActions.next(expanded);
Expand Down
8 changes: 4 additions & 4 deletions src/demo-app/expansion/expansion-demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,17 @@ <h1>matAccordion</h1>
</div>
</div>
<br>
<mat-accordion [displayMode]="displayMode" [multi]="multi"
<mat-accordion [displayMode]="displayMode" [multi]="multi" [hideToggle]="hideToggle"
class="demo-expansion-width">
<mat-expansion-panel #panel1 [hideToggle]="hideToggle">
<mat-expansion-panel #panel1>
<mat-expansion-panel-header>Section 1</mat-expansion-panel-header>
<p>This is the content text that makes sense here.</p>
</mat-expansion-panel>
<mat-expansion-panel #panel2 [hideToggle]="hideToggle" [disabled]="disabled">
<mat-expansion-panel #panel2 [disabled]="disabled">
<mat-expansion-panel-header>Section 2</mat-expansion-panel-header>
<p>This is the content text that makes sense here.</p>
</mat-expansion-panel>
<mat-expansion-panel #panel3 *ngIf="showPanel3" [hideToggle]="hideToggle">
<mat-expansion-panel #panel3 *ngIf="showPanel3">
<mat-expansion-panel-header>Section 3</mat-expansion-panel-header>
<mat-checkbox #showButtons>Reveal Buttons Below</mat-checkbox>
<mat-action-row *ngIf="showButtons.checked">
Expand Down
29 changes: 29 additions & 0 deletions src/lib/expansion/accordion.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ describe('MatAccordion', () => {
MatExpansionModule
],
declarations: [
AccordionWithHideToggle,
NestedPanel,
SetOfItems,
],
Expand Down Expand Up @@ -93,6 +94,22 @@ describe('MatAccordion', () => {

expect(innerPanel.accordion).not.toBe(outerPanel.accordion);
});

it('should update the expansion panel if hideToggle changed', () => {
const fixture = TestBed.createComponent(AccordionWithHideToggle);
const panel = fixture.debugElement.query(By.directive(MatExpansionPanel));

fixture.detectChanges();

expect(panel.nativeElement.querySelector('.mat-expansion-indicator'))
.toBeTruthy('Expected the expansion indicator to be present.');

fixture.componentInstance.hideToggle = true;
fixture.detectChanges();

expect(panel.nativeElement.querySelector('.mat-expansion-indicator'))
.toBeFalsy('Expected the expansion indicator to be removed.');
});
});


Expand Down Expand Up @@ -130,3 +147,15 @@ class NestedPanel {
@ViewChild('outerPanel') outerPanel: MatExpansionPanel;
@ViewChild('innerPanel') innerPanel: MatExpansionPanel;
}

@Component({template: `
<mat-accordion [hideToggle]="hideToggle">
<mat-expansion-panel>
<mat-expansion-panel-header>Header</mat-expansion-panel-header>
<p>Content</p>
</mat-expansion-panel>
</mat-accordion>`
})
class AccordionWithHideToggle {
hideToggle = false;
}
1 change: 1 addition & 0 deletions src/lib/expansion/accordion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export type MatAccordionDisplayMode = 'default' | 'flat';
@Directive({
selector: 'mat-accordion',
exportAs: 'matAccordion',
inputs: ['multi'],
host: {
class: 'mat-accordion'
}
Expand Down
16 changes: 10 additions & 6 deletions src/lib/expansion/expansion-panel-header.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
OnDestroy,
ViewEncapsulation,
} from '@angular/core';
import {merge, Subscription} from 'rxjs';
import {merge, Subscription, EMPTY} from 'rxjs';
import {filter} from 'rxjs/operators';
import {matExpansionAnimations} from './expansion-animations';
import {MatExpansionPanel} from './expansion-panel';
Expand Down Expand Up @@ -65,16 +65,20 @@ export class MatExpansionPanelHeader implements OnDestroy {
private _parentChangeSubscription = Subscription.EMPTY;

constructor(
@Host() public panel: MatExpansionPanel,
private _element: ElementRef,
private _focusMonitor: FocusMonitor,
private _changeDetectorRef: ChangeDetectorRef) {
@Host() public panel: MatExpansionPanel,
private _element: ElementRef,
private _focusMonitor: FocusMonitor,
private _changeDetectorRef: ChangeDetectorRef) {

const accordionHideToggleChange = panel.accordion ?
panel.accordion._stateChanges.pipe(filter(changes => !!changes.hideToggle)) : EMPTY;

// Since the toggle state depends on an @Input on the panel, we
// need to subscribe and trigger change detection manually.
// need to subscribe and trigger change detection manually.
this._parentChangeSubscription = merge(
panel.opened,
panel.closed,
accordionHideToggleChange,
panel._inputChanges.pipe(filter(changes => !!(changes.hideToggle || changes.disabled)))
)
.subscribe(() => this._changeDetectorRef.markForCheck());
Expand Down
15 changes: 6 additions & 9 deletions src/lib/expansion/expansion-panel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ export class MatExpansionPanel extends CdkAccordionItem
implements AfterContentInit, OnChanges, OnDestroy {
/** Whether the toggle indicator should be hidden. */
@Input()
get hideToggle(): boolean { return this._hideToggle; }
get hideToggle(): boolean {
return this._hideToggle || (this.accordion && this.accordion.hideToggle);
}
set hideToggle(value: boolean) {
this._hideToggle = coerceBooleanProperty(value);
}
Expand Down Expand Up @@ -109,17 +111,12 @@ export class MatExpansionPanel extends CdkAccordionItem
this.accordion = accordion;
}

/** Whether the expansion indicator should be hidden. */
_getHideToggle(): boolean {
if (this.accordion) {
return this.accordion.hideToggle;
}
return this.hideToggle;
}

/** Determines whether the expansion panel should have spacing between it and its siblings. */
_hasSpacing(): boolean {
if (this.accordion) {
// We don't need to subscribe to the `stateChanges` of the parent accordion because each time
// the [displayMode] input changes, the change detection will also cover the host bindings
// of this expansion panel.
return (this.expanded ? this.accordion.displayMode : this._getExpandedState()) === 'default';
}
return false;
Expand Down

0 comments on commit 616cb57

Please sign in to comment.