Skip to content

Commit

Permalink
fix(material/sort): view not updated when sort state is changed throu…
Browse files Browse the repository at this point in the history
…gh binding (#19492)

Fixes the visible state of the sort header not being updated if it gets changed through
the `matSortActive` binding.

Fixes #19467.
  • Loading branch information
crisbeto authored Mar 7, 2021
1 parent 9f879b2 commit ca7d379
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 25 deletions.
53 changes: 28 additions & 25 deletions src/material/sort/sort-header.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,20 +159,7 @@ export class MatSortHeader extends _MatSortHeaderMixinBase
throw getSortHeaderNotContainedWithinSortError();
}

this._rerenderSubscription = merge(_sort.sortChange, _sort._stateChanges, _intl.changes)
.subscribe(() => {
if (this._isSorted()) {
this._updateArrowDirection();
}

// If this header was recently active and now no longer sorted, animate away the arrow.
if (!this._isSorted() && this._viewState && this._viewState.toState === 'active') {
this._disableViewStateAnimation = false;
this._setAnimationTransitionState({fromState: 'active', toState: this._arrowDirection});
}

_changeDetectorRef.markForCheck();
});
this._handleStateChanges();
}

ngOnInit() {
Expand Down Expand Up @@ -243,27 +230,17 @@ export class MatSortHeader extends _MatSortHeaderMixinBase

/** Triggers the sort on this sort header and removes the indicator hint. */
_toggleOnInteraction() {

this._sort.sort(this);

// Do not show the animation if the header was already shown in the right position.
if (this._viewState.toState === 'hint' || this._viewState.toState === 'active') {
this._disableViewStateAnimation = true;
}

// If the arrow is now sorted, animate the arrow into place. Otherwise, animate it away into
// the direction it is facing.
const viewState: ArrowViewStateTransition = this._isSorted() ?
{fromState: this._arrowDirection, toState: 'active'} :
{fromState: 'active', toState: this._arrowDirection};
this._setAnimationTransitionState(viewState);

this._showIndicatorHint = false;
}

_handleClick() {
if (!this._isDisabled()) {
this._toggleOnInteraction();
this._sort.sort(this);
}
}

Expand Down Expand Up @@ -330,6 +307,32 @@ export class MatSortHeader extends _MatSortHeaderMixinBase
return !this._isDisabled() || this._isSorted();
}

/** Handles changes in the sorting state. */
private _handleStateChanges() {
this._rerenderSubscription =
merge(this._sort.sortChange, this._sort._stateChanges, this._intl.changes).subscribe(() => {
if (this._isSorted()) {
this._updateArrowDirection();

// Do not show the animation if the header was already shown in the right position.
if (this._viewState.toState === 'hint' || this._viewState.toState === 'active') {

This comment has been minimized.

Copy link
@swftvsn

swftvsn Mar 15, 2021

Contributor

This row is missing the check that is present in row 327. this._viewState && this._viewState.toState ...

this._disableViewStateAnimation = true;
}

this._setAnimationTransitionState({fromState: this._arrowDirection, toState: 'active'});
this._showIndicatorHint = false;
}

// If this header was recently active and now no longer sorted, animate away the arrow.
if (!this._isSorted() && this._viewState && this._viewState.toState === 'active') {
this._disableViewStateAnimation = false;
this._setAnimationTransitionState({fromState: 'active', toState: this._arrowDirection});
}

this._changeDetectorRef.markForCheck();
});
}

static ngAcceptInputType_disableClear: BooleanInput;
static ngAcceptInputType_disabled: BooleanInput;
}
10 changes: 10 additions & 0 deletions src/material/sort/sort.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,16 @@ describe('MatSort', () => {
component.dispatchMouseEvent('defaultA', 'mouseenter');
component.expectViewAndDirectionStates(expectedStates);
});

it('should be correct when sorting programmatically', () => {
component.active = 'defaultB';
component.direction = 'asc';
fixture.detectChanges();

expectedStates.set('defaultB', {viewState: 'asc-to-active', arrowDirection: 'active-asc'});
component.expectViewAndDirectionStates(expectedStates);
});

});

it('should be able to cycle from asc -> desc from either start point', () => {
Expand Down

0 comments on commit ca7d379

Please sign in to comment.