Skip to content

Commit

Permalink
fix(material/chips): styling not cleared on basic chips in listbox an…
Browse files Browse the repository at this point in the history
…d grid (#26771)

Fixes several issues that prevented the basic chips from appearing basic in `mat-chip-listbox` and `mat-chip-grid`:
* `basicChipAttrName` was overwritten to the correct value, but because we were checking it in the constructor, the override value wasn't picked up.
* The proper host bindings weren't in place to clear the styling.
* The user agent styles typography styles of the `button` inside the `mat-basic-chip-option` weren't cleared.
* The logic that checks if a click originates from a chip was excluding basic chips.

(cherry picked from commit d444c58)
  • Loading branch information
crisbeto committed Mar 14, 2023
1 parent e676125 commit 501d73e
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 13 deletions.
13 changes: 13 additions & 0 deletions src/dev-app/chips/chips-demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,19 @@ <h4>Unstyled</h4>
<mat-basic-chip>Basic Chip 3</mat-basic-chip>
</mat-chip-set>

<mat-chip-listbox>
<mat-basic-chip-option>Basic Chip Option 1</mat-basic-chip-option>
<mat-basic-chip-option>Basic Chip Option 2</mat-basic-chip-option>
<mat-basic-chip-option>Basic Chip Option 3</mat-basic-chip-option>
</mat-chip-listbox>

<mat-chip-grid #basicGrid>
<mat-basic-chip-row>Basic Chip Row 1</mat-basic-chip-row>
<mat-basic-chip-row>Basic Chip Row 2</mat-basic-chip-row>
<mat-basic-chip-row>Basic Chip Row 3</mat-basic-chip-row>
<input [matChipInputFor]="basicGrid" readonly>
</mat-chip-grid>

<h4>With avatar, icons, and color</h4>

<mat-chip-set>
Expand Down
2 changes: 1 addition & 1 deletion src/dev-app/chips/chips-demo.scss
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
}
}

mat-basic-chip {
.mat-mdc-basic-chip {
margin: auto 10px;
}

Expand Down
9 changes: 6 additions & 3 deletions src/material/chips/chip-option.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,10 @@ export class MatChipSelectionChange {
styleUrls: ['chip.css'],
inputs: ['color', 'disabled', 'disableRipple', 'tabIndex'],
host: {
'class':
'mat-mdc-chip mat-mdc-chip-option mdc-evolution-chip mdc-evolution-chip--filter mdc-evolution-chip--selectable',
'class': 'mat-mdc-chip mat-mdc-chip-option',
'[class.mdc-evolution-chip]': '!_isBasicChip',
'[class.mdc-evolution-chip--filter]': '!_isBasicChip',
'[class.mdc-evolution-chip--selectable]': '!_isBasicChip',
'[class.mat-mdc-chip-selected]': 'selected',
'[class.mat-mdc-chip-multiple]': '_chipListMultiple',
'[class.mat-mdc-chip-disabled]': 'disabled',
Expand Down Expand Up @@ -141,7 +143,8 @@ export class MatChipOption extends MatChip implements OnInit {
@Output() readonly selectionChange: EventEmitter<MatChipSelectionChange> =
new EventEmitter<MatChipSelectionChange>();

ngOnInit() {
override ngOnInit() {
super.ngOnInit();
this.role = 'presentation';
}

Expand Down
3 changes: 1 addition & 2 deletions src/material/chips/chip-set.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,7 @@ export class MatChipSet
let currentElement = event.target as HTMLElement | null;

while (currentElement && currentElement !== this._elementRef.nativeElement) {
// Null check the classList, because IE and Edge don't support it on all elements.
if (currentElement.classList && currentElement.classList.contains('mdc-evolution-chip')) {
if (currentElement.classList.contains('mat-mdc-chip')) {
return true;
}
currentElement = currentElement.parentElement;
Expand Down
6 changes: 6 additions & 0 deletions src/material/chips/chip.scss
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,12 @@
}
}

// Keeps basic listbox chips looking consistent with the other variations. Listbox chips don't
// inherit the font size, because they wrap the label in a `button` that has user agent styles.
.mat-mdc-basic-chip .mdc-evolution-chip__action--primary {
font: inherit;
}

// MDC's focus and hover indication is handled through their ripple which we currently
// don't use due to size concerns so we have to re-implement it ourselves.
.mat-mdc-chip-focus-overlay {
Expand Down
17 changes: 12 additions & 5 deletions src/material/chips/chip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
Attribute,
ContentChildren,
QueryList,
OnInit,
} from '@angular/core';
import {DOCUMENT} from '@angular/common';
import {
Expand Down Expand Up @@ -116,6 +117,7 @@ const _MatChipMixinBase = mixinTabIndex(
export class MatChip
extends _MatChipMixinBase
implements
OnInit,
AfterViewInit,
AfterContentInit,
CanColor,
Expand All @@ -136,7 +138,7 @@ export class MatChip
readonly _onBlur = new Subject<MatChipEvent>();

/** Whether this chip is a basic (unstyled) chip. */
readonly _isBasicChip: boolean;
_isBasicChip: boolean;

/** Role for the root of the chip. */
@Input() role: string | null = null;
Expand Down Expand Up @@ -263,18 +265,23 @@ export class MatChip
@Attribute('tabindex') tabIndex?: string,
) {
super(elementRef);
const element = elementRef.nativeElement;
this._document = _document;
this._animationsDisabled = animationMode === 'NoopAnimations';
this._isBasicChip =
element.hasAttribute(this.basicChipAttrName) ||
element.tagName.toLowerCase() === this.basicChipAttrName;
if (tabIndex != null) {
this.tabIndex = parseInt(tabIndex) ?? this.defaultTabIndex;
}
this._monitorFocus();
}

ngOnInit() {
// This check needs to happen in `ngOnInit` so the overridden value of
// `basicChipAttrName` coming from base classes can be picked up.
const element = this._elementRef.nativeElement;
this._isBasicChip =
element.hasAttribute(this.basicChipAttrName) ||
element.tagName.toLowerCase() === this.basicChipAttrName;
}

ngAfterViewInit() {
this._textElement = this._elementRef.nativeElement.querySelector('.mat-mdc-chip-action-label')!;

Expand Down
6 changes: 4 additions & 2 deletions tools/public_api_guard/material/chips.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export const MAT_CHIP_TRAILING_ICON: InjectionToken<unknown>;
export const MAT_CHIPS_DEFAULT_OPTIONS: InjectionToken<MatChipsDefaultOptions>;

// @public
export class MatChip extends _MatChipMixinBase implements AfterViewInit, AfterContentInit, CanColor, CanDisableRipple, CanDisable, HasTabIndex, OnDestroy {
export class MatChip extends _MatChipMixinBase implements OnInit, AfterViewInit, AfterContentInit, CanColor, CanDisableRipple, CanDisable, HasTabIndex, OnDestroy {
constructor(_changeDetectorRef: ChangeDetectorRef, elementRef: ElementRef<HTMLElement>, _ngZone: NgZone, _focusMonitor: FocusMonitor, _document: any, animationMode?: string, _globalRippleOptions?: RippleGlobalOptions | undefined, tabIndex?: string);
protected _allLeadingIcons: QueryList<MatChipAvatar>;
protected _allRemoveIcons: QueryList<MatChipRemove>;
Expand Down Expand Up @@ -89,7 +89,7 @@ export class MatChip extends _MatChipMixinBase implements AfterViewInit, AfterCo
// (undocumented)
protected _highlighted: boolean;
id: string;
readonly _isBasicChip: boolean;
_isBasicChip: boolean;
readonly _isRippleCentered = false;
_isRippleDisabled(): boolean;
leadingIcon: MatChipAvatar;
Expand All @@ -100,6 +100,8 @@ export class MatChip extends _MatChipMixinBase implements AfterViewInit, AfterCo
// (undocumented)
ngOnDestroy(): void;
// (undocumented)
ngOnInit(): void;
// (undocumented)
protected _ngZone: NgZone;
readonly _onBlur: Subject<MatChipEvent>;
readonly _onFocus: Subject<MatChipEvent>;
Expand Down

0 comments on commit 501d73e

Please sign in to comment.