Skip to content

Commit 9badc54

Browse files
authored
fix(chips): only add type attribute to button remove icons (#18477)
In #18095 we made it so that the chip remove button automatically adds `type="button"` so that it doesn't accidentally submit forms. It was made so that it always gets added since it was less code than adding extra checks, but the problem is that it ends up conflicting with some CSS resets. These changes make it so that we only add it for buttons. Fixes #18464.
1 parent ff8d56e commit 9badc54

File tree

5 files changed

+37
-12
lines changed

5 files changed

+37
-12
lines changed

src/material-experimental/mdc-chips/chip-icons.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,6 @@ const _MatChipRemoveMixinBase:
112112
'(click)': 'interaction.next($event)',
113113
'(keydown)': 'interaction.next($event)',
114114

115-
// Prevent accidental form submissions.
116-
'type': 'button',
117-
118115
// We need to remove this explicitly, because it gets inherited from MatChipTrailingIcon.
119116
'[attr.aria-hidden]': 'null',
120117
}
@@ -126,8 +123,12 @@ export class MatChipRemove extends _MatChipRemoveMixinBase implements CanDisable
126123
*/
127124
interaction: Subject<MouseEvent | KeyboardEvent> = new Subject<MouseEvent | KeyboardEvent>();
128125

129-
constructor(_elementRef: ElementRef) {
130-
super(_elementRef);
126+
constructor(elementRef: ElementRef) {
127+
super(elementRef);
128+
129+
if (elementRef.nativeElement.nodeName === 'BUTTON') {
130+
elementRef.nativeElement.setAttribute('type', 'button');
131+
}
131132
}
132133

133134
static ngAcceptInputType_disabled: BooleanInput;

src/material-experimental/mdc-chips/chip-remove.spec.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@ describe('MDC-based Chip Remove', () => {
4949
expect(buttonElement.getAttribute('type')).toBe('button');
5050
});
5151

52+
it('should not set the `type` attribute on non-button elements', () => {
53+
const buttonElement = chipNativeElement.querySelector('span.mat-mdc-chip-remove')!;
54+
55+
expect(buttonElement.hasAttribute('type')).toBe(false);
56+
});
57+
5258
it('should start MDC exit animation on click', () => {
5359
let buttonElement = chipNativeElement.querySelector('button')!;
5460

@@ -163,7 +169,10 @@ describe('MDC-based Chip Remove', () => {
163169
<mat-chip
164170
[removable]="removable"
165171
[disabled]="disabled"
166-
(removed)="didRemove()"><button matChipRemove></button></mat-chip>
172+
(removed)="didRemove()">
173+
<button matChipRemove></button>
174+
<span matChipRemove></span>
175+
</mat-chip>
167176
`
168177
})
169178
class TestChip {

src/material/chips/chip-remove.spec.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ describe('Chip Remove', () => {
4242
expect(buttonElement.getAttribute('type')).toBe('button');
4343
});
4444

45+
it('should not set the `type` attribute on non-button elements', () => {
46+
const buttonElement = chipNativeElement.querySelector('span.mat-chip-remove')!;
47+
48+
expect(buttonElement.hasAttribute('type')).toBe(false);
49+
});
50+
4551
it('should emits (removed) on click', () => {
4652
let buttonElement = chipNativeElement.querySelector('button')!;
4753

@@ -79,7 +85,10 @@ describe('Chip Remove', () => {
7985
<mat-chip
8086
[removable]="removable"
8187
[disabled]="disabled"
82-
(removed)="didRemove()"><button matChipRemove></button></mat-chip>
88+
(removed)="didRemove()">
89+
<button matChipRemove></button>
90+
<span matChipRemove></span>
91+
</mat-chip>
8392
`
8493
})
8594
class TestChip {

src/material/chips/chip.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -439,13 +439,19 @@ export class MatChip extends _MatChipMixinBase implements FocusableOption, OnDes
439439
host: {
440440
'class': 'mat-chip-remove mat-chip-trailing-icon',
441441
'(click)': '_handleClick($event)',
442-
443-
// Prevent accidental form submissions.
444-
'type': 'button',
445442
}
446443
})
447444
export class MatChipRemove {
448-
constructor(protected _parentChip: MatChip) {}
445+
constructor(
446+
protected _parentChip: MatChip,
447+
// @breaking-change 11.0.0 `elementRef` parameter to be made required.
448+
elementRef?: ElementRef<HTMLElement>) {
449+
450+
// @breaking-change 11.0.0 Remove null check for `elementRef`.
451+
if (elementRef && elementRef.nativeElement.nodeName === 'BUTTON') {
452+
elementRef.nativeElement.setAttribute('type', 'button');
453+
}
454+
}
449455

450456
/** Calls the parent chip's public `remove()` method if applicable. */
451457
_handleClick(event: Event): void {

tools/public_api_guard/material/chips.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ export declare class MatChipListChange {
182182

183183
export declare class MatChipRemove {
184184
protected _parentChip: MatChip;
185-
constructor(_parentChip: MatChip);
185+
constructor(_parentChip: MatChip, elementRef?: ElementRef<HTMLElement>);
186186
_handleClick(event: Event): void;
187187
static ɵdir: i0.ɵɵDirectiveDefWithMeta<MatChipRemove, "[matChipRemove]", never, {}, {}, never>;
188188
static ɵfac: i0.ɵɵFactoryDef<MatChipRemove>;

0 commit comments

Comments
 (0)