Skip to content

Commit 5b7e422

Browse files
authored
fix(radio,toggle,checkbox,select): padded space is clickable in items (#28136)
Issue number: Resolves #27169 --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> Clicking the padded space within an `ion-item` will not pass the click event to the slotted `ion-radio`, `ion-checkbox`, `ion-select` or `ion-toggle`. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - The padded space at the start of `.item-native` and at the end of `.item-inner` is clickable to activate a control. - When the item is clicked, we check if the event is a result of clicking the control or clicking the item's padded space. If the click event is on the control, we don't need to do anything and let the default behavior occur. If the click event is on the padded space, we manually call the `.click()` method for the interactive element. - The cursor pointer displays when hovering over the padded space when a slotted interactive control is present. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. -->
1 parent 0104d89 commit 5b7e422

File tree

9 files changed

+205
-11
lines changed

9 files changed

+205
-11
lines changed

core/src/components/checkbox/checkbox.tsx

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,8 @@ export class Checkbox implements ComponentInterface {
140140
*/
141141
@Event() ionStyle!: EventEmitter<StyleEventDetail>;
142142

143-
// TODO(FW-3100): remove this
144143
connectedCallback() {
145-
this.legacyFormController = createLegacyFormController(this.el);
144+
this.legacyFormController = createLegacyFormController(this.el); // TODO(FW-3100): remove this
146145
}
147146

148147
componentWillLoad() {
@@ -195,7 +194,7 @@ export class Checkbox implements ComponentInterface {
195194
});
196195
};
197196

198-
private toggleChecked = (ev: any) => {
197+
private toggleChecked = (ev: Event) => {
199198
ev.preventDefault();
200199

201200
this.setFocus();
@@ -211,6 +210,10 @@ export class Checkbox implements ComponentInterface {
211210
this.ionBlur.emit();
212211
};
213212

213+
private onClick = (ev: MouseEvent) => {
214+
this.toggleChecked(ev);
215+
};
216+
214217
// TODO(FW-3100): run contents of renderCheckbox directly instead
215218
render() {
216219
const { legacyFormController } = this;
@@ -252,6 +255,7 @@ export class Checkbox implements ComponentInterface {
252255
[`checkbox-alignment-${alignment}`]: true,
253256
[`checkbox-label-placement-${labelPlacement}`]: true,
254257
})}
258+
onClick={this.onClick}
255259
>
256260
<label class="checkbox-wrapper">
257261
{/*
@@ -333,6 +337,7 @@ Developers can dismiss this warning by removing their usage of the "legacy" prop
333337
'legacy-checkbox': true,
334338
interactive: true,
335339
})}
340+
onClick={this.onClick}
336341
>
337342
<svg class="checkbox-icon" viewBox="0 0 24 24" part="container">
338343
{path}

core/src/components/checkbox/test/basic/checkbox.e2e.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,5 +83,28 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
8383
await checkbox.evaluate((el: HTMLIonCheckboxElement) => (el.checked = true));
8484
expect(ionChange).not.toHaveReceivedEvent();
8585
});
86+
87+
test('clicking padded space within item should click the checkbox', async ({ page }) => {
88+
await page.setContent(
89+
`
90+
<ion-item>
91+
<ion-checkbox>Size</ion-checkbox>
92+
</ion-item>
93+
`,
94+
config
95+
);
96+
const itemNative = page.locator('.item-native');
97+
const ionChange = await page.spyOnEvent('ionChange');
98+
99+
// Clicks the padded space within the item
100+
await itemNative.click({
101+
position: {
102+
x: 5,
103+
y: 5,
104+
},
105+
});
106+
107+
expect(ionChange).toHaveReceivedEvent();
108+
});
86109
});
87110
});

core/src/components/checkbox/test/legacy/basic/checkbox.e2e.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,5 +67,28 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => {
6767
await checkbox.evaluate((el: HTMLIonCheckboxElement) => (el.checked = true));
6868
await expect(ionChange).not.toHaveReceivedEvent();
6969
});
70+
71+
test('clicking padded space within item should click the checkbox', async ({ page }) => {
72+
await page.setContent(
73+
`
74+
<ion-item>
75+
<ion-checkbox legacy="true" value="my-checkbox"></ion-checkbox>
76+
</ion-item>
77+
`,
78+
config
79+
);
80+
const itemNative = page.locator('.item-native');
81+
const ionChange = await page.spyOnEvent('ionChange');
82+
83+
// Clicks the padded space within the item
84+
await itemNative.click({
85+
position: {
86+
x: 5,
87+
y: 5,
88+
},
89+
});
90+
91+
expect(ionChange).toHaveReceivedEvent();
92+
});
7093
});
7194
});

core/src/components/item/item.scss

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,13 @@
173173
}
174174
}
175175

176+
// Item: Interactive
177+
// --------------------------------------------------
178+
179+
:host(.item-has-interactive-control) {
180+
cursor: pointer;
181+
}
182+
176183

177184
// Item: Disabled
178185
// --------------------------------------------------
@@ -189,6 +196,7 @@
189196
}
190197

191198

199+
192200
// Native Item
193201
// --------------------------------------------------
194202

core/src/components/item/item.tsx

Lines changed: 51 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { ComponentInterface } from '@stencil/core';
2-
import { Component, Element, Host, Listen, Prop, State, Watch, forceUpdate, h } from '@stencil/core';
2+
import { Build, Component, Element, Host, Listen, Prop, State, Watch, forceUpdate, h } from '@stencil/core';
33
import type { AnchorInterface, ButtonInterface } from '@utils/element-interface';
44
import type { Attributes } from '@utils/helpers';
55
import { inheritAttributes, raf } from '@utils/helpers';
@@ -350,6 +350,22 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
350350
}
351351
}
352352

353+
private getFirstInteractive() {
354+
if (Build.isTesting) {
355+
/**
356+
* Pseudo selectors can't be tested in unit tests.
357+
* It will cause an error when running the tests.
358+
*
359+
* TODO: FW-5205 - Remove the build conditional when this is fixed in Stencil
360+
*/
361+
return undefined;
362+
}
363+
const controls = this.el.querySelectorAll<HTMLElement>(
364+
'ion-toggle:not([disabled]), ion-checkbox:not([disabled]), ion-radio:not([disabled]), ion-select:not([disabled])'
365+
);
366+
return controls[0];
367+
}
368+
353369
render() {
354370
const {
355371
counterString,
@@ -367,6 +383,7 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
367383
routerAnimation,
368384
routerDirection,
369385
inheritedAriaAttributes,
386+
multipleInputs,
370387
} = this;
371388
const childStyles = {} as StyleEventDetail;
372389
const mode = getIonMode(this);
@@ -383,15 +400,41 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
383400
rel,
384401
target,
385402
};
403+
404+
let clickFn = {};
405+
406+
const firstInteractive = this.getFirstInteractive();
407+
386408
// Only set onClick if the item is clickable to prevent screen
387409
// readers from reading all items as clickable
388-
const clickFn = clickable
389-
? {
390-
onClick: (ev: Event) => {
410+
if (clickable || (firstInteractive !== undefined && !multipleInputs)) {
411+
clickFn = {
412+
onClick: (ev: MouseEvent) => {
413+
if (clickable) {
391414
openURL(href, ev, routerDirection, routerAnimation);
392-
},
393-
}
394-
: {};
415+
}
416+
if (firstInteractive !== undefined && !multipleInputs) {
417+
const path = ev.composedPath();
418+
const target = path[0] as HTMLElement;
419+
if (ev.isTrusted) {
420+
/**
421+
* Dispatches a click event to the first interactive element,
422+
* when it is the result of a user clicking on the item.
423+
*
424+
* We check if the click target is in the shadow root,
425+
* which means the user clicked on the .item-native or
426+
* .item-inner padding.
427+
*/
428+
const clickedWithinShadowRoot = this.el.shadowRoot!.contains(target);
429+
if (clickedWithinShadowRoot) {
430+
firstInteractive.click();
431+
}
432+
}
433+
}
434+
},
435+
};
436+
}
437+
395438
const showDetail = detail !== undefined ? detail : mode === 'ios' && clickable;
396439
this.itemStyles.forEach((value) => {
397440
Object.assign(childStyles, value);
@@ -413,6 +456,7 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
413456
[`item-lines-${lines}`]: lines !== undefined,
414457
[`item-fill-${fillValue}`]: true,
415458
[`item-shape-${shape}`]: shape !== undefined,
459+
'item-has-interactive-control': firstInteractive !== undefined,
416460
'item-disabled': disabled,
417461
'in-list': inList,
418462
'item-multiple-inputs': this.multipleInputs,

core/src/components/radio/test/item/radio.e2e.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,4 +78,33 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, co
7878
await expect(list).toHaveScreenshot(screenshot(`radio-stacked-label-in-item`));
7979
});
8080
});
81+
82+
test.describe(title('radio: ionChange'), () => {
83+
test('clicking padded space within item should click the radio', async ({ page }) => {
84+
await page.setContent(
85+
`
86+
<ion-radio-group>
87+
<ion-item>
88+
<ion-radio>
89+
<ion-label>Enable Notifications</ion-label>
90+
</ion-radio>
91+
</ion-item>
92+
</ion-radio-group>
93+
`,
94+
config
95+
);
96+
const itemNative = page.locator('.item-native');
97+
const ionChange = await page.spyOnEvent('ionChange');
98+
99+
// Clicks the padded space within the item
100+
await itemNative.click({
101+
position: {
102+
x: 5,
103+
y: 5,
104+
},
105+
});
106+
107+
expect(ionChange).toHaveReceivedEvent();
108+
});
109+
});
81110
});

core/src/components/select/test/basic/select.e2e.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,5 +234,33 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
234234
await select.evaluate((el: HTMLIonSelectElement) => (el.value = 'banana'));
235235
await expect(ionChange).not.toHaveReceivedEvent();
236236
});
237+
238+
test('clicking padded space within item should click the select', async ({ page }) => {
239+
await page.setContent(
240+
`
241+
<ion-item>
242+
<ion-select label="Fruit" interface="action-sheet">
243+
<ion-select-option value="apple">Apple</ion-select-option>
244+
<ion-select-option value="banana">Banana</ion-select-option>
245+
</ion-select>
246+
</ion-item>
247+
`,
248+
config
249+
);
250+
const itemNative = page.locator('.item-native');
251+
const ionActionSheetDidPresent = await page.spyOnEvent('ionActionSheetDidPresent');
252+
253+
// Clicks the padded space within the item
254+
await itemNative.click({
255+
position: {
256+
x: 5,
257+
y: 5,
258+
},
259+
});
260+
261+
await ionActionSheetDidPresent.next();
262+
263+
expect(ionActionSheetDidPresent).toHaveReceivedEvent();
264+
});
237265
});
238266
});

core/src/components/toggle/test/item/toggle.e2e.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,4 +89,38 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, co
8989
await expect(list).toHaveScreenshot(screenshot(`toggle-stacked-label-in-item`));
9090
});
9191
});
92+
93+
test.describe(title('toggle: ionChange'), () => {
94+
test('clicking padded space within item should click the toggle', async ({ page }) => {
95+
await page.setContent(
96+
`
97+
<ion-item>
98+
<ion-toggle>Size</ion-toggle>
99+
</ion-item>
100+
`,
101+
config
102+
);
103+
const itemNative = page.locator('.item-native');
104+
const ionChange = await page.spyOnEvent('ionChange');
105+
106+
/**
107+
* Clicks the padded space within the item.
108+
*
109+
* We intentionally activate the toggle control when clicking either
110+
* the label or padded space. This is different than native iOS,
111+
* but we do it for three reasons:
112+
* 1. Clicking a label connected to a control is standard behavior for web controls.
113+
* 2. iOS is inconsistent in their implementation and other controls can be activated by clicking the label.
114+
* 3. MD is consistent in their implementation and activates controls by clicking the label.
115+
*/
116+
await itemNative.click({
117+
position: {
118+
x: 5,
119+
y: 5,
120+
},
121+
});
122+
123+
expect(ionChange).toHaveReceivedEvent();
124+
});
125+
});
92126
});

core/src/components/toggle/toggle.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ export class Toggle implements ComponentInterface {
258258
}
259259
}
260260

261-
private onClick = (ev: Event) => {
261+
private onClick = (ev: MouseEvent) => {
262262
ev.preventDefault();
263263

264264
if (this.lastDrag + 300 < Date.now()) {

0 commit comments

Comments
 (0)