Skip to content
This repository was archived by the owner on Jan 13, 2025. It is now read-only.

Commit 6faaabe

Browse files
authored
chore: Fix regression in MDCChipFoundation.getDimensions() (#4443)
PR #4332 inadvertently broke short-circuit logic and evaluation order. The change caused incorrect ripple size calculations on filter chips without leading icons, which was caught by internal screenshot tests.
1 parent b6b6983 commit 6faaabe

File tree

2 files changed

+26
-11
lines changed

2 files changed

+26
-11
lines changed

packages/mdc-checkbox/component.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,16 @@ import {MDCRipple, MDCRippleAdapter, MDCRippleCapableSurface, MDCRippleFoundatio
2828
import {MDCCheckboxAdapter} from './adapter';
2929
import {MDCCheckboxFoundation} from './foundation';
3030

31+
/**
32+
* This type is needed for compatibility with Closure Compiler.
33+
*/
34+
type PropertyDescriptorGetter = (() => any) | undefined; // tslint:disable-line:no-any
35+
3136
const {NATIVE_CONTROL_SELECTOR} = MDCCheckboxFoundation.strings;
3237

3338
const CB_PROTO_PROPS = ['checked', 'indeterminate'];
3439

3540
export class MDCCheckbox extends MDCComponent<MDCCheckboxFoundation> implements MDCRippleCapableSurface {
36-
3741
static attachTo(root: Element) {
3842
return new MDCCheckbox(root);
3943
}
@@ -115,10 +119,13 @@ export class MDCCheckbox extends MDCComponent<MDCCheckboxFoundation> implements
115119
return;
116120
}
117121

122+
// Type cast is needed for compatibility with Closure Compiler.
123+
const nativeGetter = (desc as {get: PropertyDescriptorGetter}).get;
124+
118125
const nativeCbDesc = {
119126
configurable: desc.configurable,
120127
enumerable: desc.enumerable,
121-
get: desc.get,
128+
get: nativeGetter,
122129
set: (state: boolean) => {
123130
desc.set!.call(nativeCb, state);
124131
this.foundation_.handleChange();

packages/mdc-chips/chip/foundation.ts

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -94,19 +94,27 @@ export class MDCChipFoundation extends MDCFoundation<MDCChipAdapter> {
9494
}
9595

9696
getDimensions(): ClientRect {
97-
const rootRect = this.adapter_.getRootBoundingClientRect();
98-
const checkmarkRect = this.adapter_.getCheckmarkBoundingClientRect();
97+
const getRootRect = () => this.adapter_.getRootBoundingClientRect();
98+
const getCheckmarkRect = () => this.adapter_.getCheckmarkBoundingClientRect();
9999

100100
// When a chip has a checkmark and not a leading icon, the bounding rect changes in size depending on the current
101101
// size of the checkmark.
102-
if (!this.adapter_.hasLeadingIcon() && checkmarkRect !== null) {
103-
// The checkmark's width is initially set to 0, so use the checkmark's height as a proxy since the checkmark
104-
// should always be square.
105-
const width = rootRect.width + checkmarkRect.height;
106-
return {...rootRect, width};
107-
} else {
108-
return rootRect;
102+
if (!this.adapter_.hasLeadingIcon()) {
103+
const checkmarkRect = getCheckmarkRect();
104+
if (checkmarkRect) {
105+
const rootRect = getRootRect();
106+
const height = rootRect.height;
107+
// Checkmark is a square, meaning the client rect's width and height are identical once the animation completes.
108+
// However, the checkbox is initially hidden by setting the width to 0.
109+
// To account for an initial width of 0, we use the checkbox's height instead (which equals the end-state width)
110+
// when adding it to the root client rect's width.
111+
const checkmarkWidth = checkmarkRect.height;
112+
const width = rootRect.width + checkmarkWidth;
113+
return {...rootRect, width, height};
114+
}
109115
}
116+
117+
return getRootRect();
110118
}
111119

112120
/**

0 commit comments

Comments
 (0)