Skip to content

Commit

Permalink
fix(multiple): stop exposing internal ripple implementation (#29622)
Browse files Browse the repository at this point in the history
Removes the code that exposes the ripple implementations of some components since they are internal details and they require some hacky workarounds to keep exposed.

BREAKING CHANGE:
* `MatButton.ripple` is no longer available.
* `MatCheckbox.ripple` is no longer available.
* `MatChip.ripple` is no longer available.
  • Loading branch information
crisbeto authored Aug 22, 2024
1 parent 0fbfc8a commit 485bd99
Show file tree
Hide file tree
Showing 11 changed files with 6 additions and 186 deletions.
18 changes: 3 additions & 15 deletions src/material/button/button-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {
OnDestroy,
OnInit,
} from '@angular/core';
import {MatRipple, MatRippleLoader, ThemePalette} from '@angular/material/core';
import {MatRippleLoader, ThemePalette} from '@angular/material/core';

/** Object that can be used to configure the default options for the button component. */
export interface MatButtonConfig {
Expand Down Expand Up @@ -93,22 +93,10 @@ export class MatButtonBase implements AfterViewInit, OnDestroy {
* Handles the lazy creation of the MatButton ripple.
* Used to improve initial load time of large applications.
*/
_rippleLoader: MatRippleLoader = inject(MatRippleLoader);
protected _rippleLoader: MatRippleLoader = inject(MatRippleLoader);

/** Whether this button is a FAB. Used to apply the correct class on the ripple. */
_isFab = false;

/**
* Reference to the MatRipple instance of the button.
* @deprecated Considered an implementation detail. To be removed.
* @breaking-change 17.0.0
*/
get ripple(): MatRipple {
return this._rippleLoader?.getRipple(this._elementRef.nativeElement)!;
}
set ripple(v: MatRipple) {
this._rippleLoader?.attachRipple(this._elementRef.nativeElement, v);
}
protected _isFab = false;

/**
* Theme color of the button. This API is supported in M2 themes only, it has
Expand Down
93 changes: 2 additions & 91 deletions src/material/button/button.spec.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import {createMouseEvent, dispatchEvent} from '@angular/cdk/testing/private';
import {ApplicationRef, Component, DebugElement} from '@angular/core';
import {ApplicationRef, Component} from '@angular/core';
import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing';
import {MatRipple, ThemePalette} from '@angular/material/core';
import {ThemePalette} from '@angular/material/core';
import {By} from '@angular/platform-browser';
import {
MAT_BUTTON_CONFIG,
MAT_FAB_DEFAULT_OPTIONS,
MatButton,
MatButtonModule,
MatFabDefaultOptions,
} from './index';
Expand Down Expand Up @@ -62,14 +61,6 @@ describe('MatButton', () => {
expect(anchor.classList).toContain('mat-mdc-button-disabled');
});

it('should expose the ripple instance', () => {
const fixture = TestBed.createComponent(TestApp);
fixture.detectChanges();

const button = fixture.debugElement.query(By.directive(MatButton))!.componentInstance;
expect(button.ripple).toBeTruthy();
});

it('should not clear previous defined classes', () => {
let fixture = TestBed.createComponent(TestApp);
let testComponent = fixture.debugElement.componentInstance;
Expand Down Expand Up @@ -287,86 +278,6 @@ describe('MatButton', () => {
});
});

// Ripple tests.
describe('button ripples', () => {
let fixture: ComponentFixture<TestApp>;
let testComponent: TestApp;
let buttonDebugElement: DebugElement;
let buttonRippleInstance: MatRipple;
let anchorDebugElement: DebugElement;
let anchorRippleInstance: MatRipple;

beforeEach(() => {
fixture = TestBed.createComponent(TestApp);
fixture.detectChanges();

testComponent = fixture.componentInstance;

buttonDebugElement = fixture.debugElement.query(By.css('button[mat-button]'))!;
buttonRippleInstance = buttonDebugElement.componentInstance.ripple;

anchorDebugElement = fixture.debugElement.query(By.css('a[mat-button]'))!;
anchorRippleInstance = anchorDebugElement.componentInstance.ripple;
});

it('should disable the ripple if matRippleDisabled input is set', () => {
expect(buttonRippleInstance.disabled).toBeFalsy();

testComponent.rippleDisabled = true;
fixture.changeDetectorRef.markForCheck();
fixture.detectChanges();

expect(buttonRippleInstance.disabled).toBeTruthy();
});

it('should disable the ripple when the button is disabled', () => {
expect(buttonRippleInstance.disabled).toBeFalsy(
'Expected an enabled button[mat-button] to have an enabled ripple',
);
expect(anchorRippleInstance.disabled).toBeFalsy(
'Expected an enabled a[mat-button] to have an enabled ripple',
);

testComponent.isDisabled = true;
fixture.changeDetectorRef.markForCheck();
fixture.detectChanges();

expect(buttonRippleInstance.disabled).toBeTruthy(
'Expected a disabled button[mat-button] not to have an enabled ripple',
);
expect(anchorRippleInstance.disabled).toBeTruthy(
'Expected a disabled a[mat-button] not to have an enabled ripple',
);
});

it('should render the ripple once it is referenced', () => {
const fab = fixture.debugElement.query(By.css('button[mat-fab]'))!;
let ripple = fab.nativeElement.querySelector('.mat-mdc-button-ripple');
expect(ripple).withContext('Expect ripple to be absent before user interaction').toBeNull();

// Referencing the ripple should instantiate the ripple.
expect(fab.componentInstance.ripple).toBeDefined();

ripple = fab.nativeElement.querySelector('.mat-mdc-button-ripple');
expect(ripple)
.withContext('Expect ripple to be present after user interaction')
.not.toBeNull();
});

// Ensure each of these events triggers the initialization of the button ripple.
for (const event of ['mousedown', 'touchstart', 'mouseenter', 'focus']) {
it(`should render the ripple once a button has received a "${event}" event`, () => {
const fab = fixture.debugElement.query(By.css('button[mat-fab]'))!;
let ripple = fab.nativeElement.querySelector('.mat-mdc-button-ripple');
expect(ripple).toBeNull();

dispatchEvent(fab.nativeElement, createMouseEvent(event));
ripple = fab.nativeElement.querySelector('.mat-mdc-button-ripple');
expect(ripple).not.toBeNull();
});
}
});

it('should have a focus indicator', () => {
const fixture = TestBed.createComponent(TestApp);
const buttonNativeElements = [
Expand Down
4 changes: 0 additions & 4 deletions src/material/checkbox/checkbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,6 @@ describe('MatCheckbox', () => {
expect(inputElement.checked).toBe(false);
}));

it('should expose the ripple instance', () => {
expect(checkboxInstance.ripple).toBeTruthy();
});

it('should hide the internal SVG', () => {
const svg = checkboxNativeElement.querySelector('svg')!;
expect(svg.getAttribute('aria-hidden')).toBe('true');
Expand Down
7 changes: 0 additions & 7 deletions src/material/checkbox/checkbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,13 +216,6 @@ export class MatCheckbox
@Input({transform: booleanAttribute})
disabledInteractive: boolean;

/**
* Reference to the MatRipple instance of the checkbox.
* @deprecated Considered an implementation detail. To be removed.
* @breaking-change 17.0.0
*/
@ViewChild(MatRipple) ripple: MatRipple;

/**
* Called when the checkbox is blurred. Needed to properly implement ControlValueAccessor.
* @docs-private
Expand Down
38 changes: 0 additions & 38 deletions src/material/chips/chip.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,6 @@ describe('MatChip', () => {

expect(chip.getAttribute('tabindex')).toBe('15');
});

it('should have its ripple disabled', () => {
fixture = TestBed.createComponent(BasicChip);
fixture.detectChanges();
chipDebugElement = fixture.debugElement.query(By.directive(MatChip))!;
chipInstance = chipDebugElement.injector.get<MatChip>(MatChip);
expect(chipInstance.ripple.disabled)
.withContext('Expected basic chip ripples to be disabled.')
.toBe(true);
});
});

describe('MatChip', () => {
Expand Down Expand Up @@ -131,34 +121,6 @@ describe('MatChip', () => {
expect(testComponent.chipRemove).toHaveBeenCalledWith({chip: chipInstance});
});

it('should be able to disable ripples with the `[rippleDisabled]` input', () => {
expect(chipInstance.ripple.disabled)
.withContext('Expected chip ripples to be enabled.')
.toBe(false);

testComponent.rippleDisabled = true;
fixture.changeDetectorRef.markForCheck();
fixture.detectChanges();

expect(chipInstance.ripple.disabled)
.withContext('Expected chip ripples to be disabled.')
.toBe(true);
});

it('should disable ripples when the chip is disabled', () => {
expect(chipInstance.ripple.disabled)
.withContext('Expected chip ripples to be enabled.')
.toBe(false);

testComponent.disabled = true;
fixture.changeDetectorRef.markForCheck();
fixture.detectChanges();

expect(chipInstance.ripple.disabled)
.withContext('Expected chip ripples to be disabled.')
.toBe(true);
});

it('should make disabled chips non-focusable', () => {
testComponent.disabled = true;
fixture.changeDetectorRef.markForCheck();
Expand Down
15 changes: 1 addition & 14 deletions src/material/chips/chip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import {
} from '@angular/core';
import {
MAT_RIPPLE_GLOBAL_OPTIONS,
MatRipple,
MatRippleLoader,
RippleGlobalOptions,
} from '@angular/material/core';
Expand Down Expand Up @@ -216,26 +215,14 @@ export class MatChip implements OnInit, AfterViewInit, AfterContentInit, DoCheck
/** The chip's trailing remove icon. */
@ContentChild(MAT_CHIP_REMOVE) removeIcon: MatChipRemove;

/**
* Reference to the MatRipple instance of the chip.
* @deprecated Considered an implementation detail. To be removed.
* @breaking-change 17.0.0
*/
get ripple(): MatRipple {
return this._rippleLoader?.getRipple(this._elementRef.nativeElement)!;
}
set ripple(v: MatRipple) {
this._rippleLoader?.attachRipple(this._elementRef.nativeElement, v);
}

/** Action receiving the primary set of user interactions. */
@ViewChild(MatChipAction) primaryAction: MatChipAction;

/**
* Handles the lazy creation of the MatChip ripple.
* Used to improve initial load time of large applications.
*/
_rippleLoader: MatRippleLoader = inject(MatRippleLoader);
private _rippleLoader: MatRippleLoader = inject(MatRippleLoader);

protected _injector = inject(Injector);

Expand Down
6 changes: 0 additions & 6 deletions src/material/core/private/ripple-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,6 @@ export class MatRippleLoader implements OnDestroy {
}
}

/** Returns the ripple instance for the given host element. */
getRipple(host: HTMLElement): MatRipple | undefined {
const ripple = this._hosts.get(host);
return ripple || this._createRipple(host);
}

/** Sets the disabled state on the ripple instance corresponding to the given host element. */
setDisabled(host: HTMLElement, disabled: boolean): void {
const ripple = this._hosts.get(host);
Expand Down
1 change: 0 additions & 1 deletion tools/public_api_guard/material/button.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { FocusOrigin } from '@angular/cdk/a11y';
import * as i0 from '@angular/core';
import * as i1 from '@angular/material/core';
import { InjectionToken } from '@angular/core';
import { MatRipple } from '@angular/material/core';
import { MatRippleLoader } from '@angular/material/core';
import { NgZone } from '@angular/core';
import { OnDestroy } from '@angular/core';
Expand Down
3 changes: 0 additions & 3 deletions tools/public_api_guard/material/checkbox.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import { FocusableOption } from '@angular/cdk/a11y';
import * as i0 from '@angular/core';
import * as i3 from '@angular/material/core';
import { InjectionToken } from '@angular/core';
import { MatRipple } from '@angular/material/core';
import { NgZone } from '@angular/core';
import { OnChanges } from '@angular/core';
import { Provider } from '@angular/core';
Expand Down Expand Up @@ -114,8 +113,6 @@ export class MatCheckbox implements AfterViewInit, OnChanges, ControlValueAccess
// (undocumented)
registerOnValidatorChange(fn: () => void): void;
required: boolean;
// @deprecated
ripple: MatRipple;
// (undocumented)
setDisabledState(isDisabled: boolean): void;
tabIndex: number;
Expand Down
6 changes: 0 additions & 6 deletions tools/public_api_guard/material/chips.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import { InjectionToken } from '@angular/core';
import { Injector } from '@angular/core';
import { MatFormField } from '@angular/material/form-field';
import { MatFormFieldControl } from '@angular/material/form-field';
import { MatRipple } from '@angular/material/core';
import { MatRippleLoader } from '@angular/material/core';
import { NgControl } from '@angular/forms';
import { NgForm } from '@angular/forms';
import { NgZone } from '@angular/core';
Expand Down Expand Up @@ -116,10 +114,6 @@ export class MatChip implements OnInit, AfterViewInit, AfterContentInit, DoCheck
remove(): void;
readonly removed: EventEmitter<MatChipEvent>;
removeIcon: MatChipRemove;
// @deprecated
get ripple(): MatRipple;
set ripple(v: MatRipple);
_rippleLoader: MatRippleLoader;
role: string | null;
trailingIcon: MatChipTrailingIcon;
get value(): any;
Expand Down
1 change: 0 additions & 1 deletion tools/public_api_guard/material/core.md
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,6 @@ export class MatRippleLoader implements OnDestroy {
}): void;
// (undocumented)
destroyRipple(host: HTMLElement): void;
getRipple(host: HTMLElement): MatRipple | undefined;
// (undocumented)
ngOnDestroy(): void;
setDisabled(host: HTMLElement, disabled: boolean): void;
Expand Down

0 comments on commit 485bd99

Please sign in to comment.