Skip to content

Commit 7a9d138

Browse files
brandyscarneybrandyscarney
authored
fix(item): emit click event once when clicking padded space on item and emit correct element (#30373)
Issue number: resolves #29758 resolves #29761 --------- ## What is the current behavior? When an `ion-item` has a click event listener, the following issues occur: 1. **Double Click Events**: - Clicking the padding around interactive elements (`ion-checkbox`, `ion-toggle`, `ion-radio`, `ion-textarea`, `ion-input`) triggers the click event twice. 2. **Incorrect Event Targets**: - For `ion-input` and `ion-textarea`, clicking their native inputs reports the wrong element as the event target. - Clicking the padding within the `native-wrapper` of `ion-input` emits a separate click event with an incorrect target element. ## What is the new behavior? - Fires `firstInteractive.click()` in Item for all interactives (no longer excludes input/textarea). - Stops immediate propagation in item when the click event is in the padding of an item, preventing two click events from firing. - Updates input and textarea to always emit from their host elements `ion-input`/`ion-textarea` instead of the native input elements. - Updates input to make the native input take up 100% height. This is necessary to avoid the `native-wrapper` triggering its own click event when clicking on its padding. - Adds e2e tests to check for the above behavior to avoid future regressions. ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information **Dev build**: `8.5.6-dev.11745613928.16440384` **Previews**: - [Checkbox Preview](https://ionic-framework-git-fw-6503-ionic1.vercel.app/src/components/checkbox/test/item) - [Input Preview](https://ionic-framework-git-fw-6503-ionic1.vercel.app/src/components/input/test/item) - [Radio Preview](https://ionic-framework-git-fw-6503-ionic1.vercel.app/src/components/radio/test/item) - [Select Preview](https://ionic-framework-git-fw-6503-ionic1.vercel.app/src/components/select/test/item) - [Textarea Preview](https://ionic-framework-git-fw-6503-ionic1.vercel.app/src/components/textarea/test/item) - [Toggle Preview](https://ionic-framework-git-fw-6503-ionic1.vercel.app/src/components/toggle/test/item) --------- Co-authored-by: Brandy Smith <[email protected]>
1 parent 8ef79cf commit 7a9d138

File tree

12 files changed

+437
-64
lines changed

12 files changed

+437
-64
lines changed

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

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -98,28 +98,5 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
9898
await checkbox.evaluate((el: HTMLIonCheckboxElement) => (el.checked = true));
9999
expect(ionChange).not.toHaveReceivedEvent();
100100
});
101-
102-
test('clicking padded space within item should click the checkbox', async ({ page }) => {
103-
await page.setContent(
104-
`
105-
<ion-item>
106-
<ion-checkbox>Size</ion-checkbox>
107-
</ion-item>
108-
`,
109-
config
110-
);
111-
const itemNative = page.locator('.item-native');
112-
const ionChange = await page.spyOnEvent('ionChange');
113-
114-
// Clicks the padded space within the item
115-
await itemNative.click({
116-
position: {
117-
x: 5,
118-
y: 5,
119-
},
120-
});
121-
122-
expect(ionChange).toHaveReceivedEvent();
123-
});
124101
});
125102
});

‎core/src/components/checkbox/test/item/checkbox.e2e.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,3 +127,70 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
127127
});
128128
});
129129
});
130+
131+
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
132+
test.describe(title('checkbox: item functionality'), () => {
133+
test('clicking padded space within item should click the checkbox', async ({ page }) => {
134+
test.info().annotations.push({
135+
type: 'issue',
136+
description: 'https://github.com/ionic-team/ionic-framework/issues/27169',
137+
});
138+
139+
await page.setContent(
140+
`
141+
<ion-item>
142+
<ion-checkbox>Size</ion-checkbox>
143+
</ion-item>
144+
`,
145+
config
146+
);
147+
const item = page.locator('ion-item');
148+
const ionChange = await page.spyOnEvent('ionChange');
149+
150+
// Clicks the padded space within the item
151+
await item.click({
152+
position: {
153+
x: 5,
154+
y: 5,
155+
},
156+
});
157+
158+
expect(ionChange).toHaveReceivedEvent();
159+
});
160+
161+
test('clicking padded space within item should fire one click event', async ({ page }) => {
162+
test.info().annotations.push({
163+
type: 'issue',
164+
description: 'https://github.com/ionic-team/ionic-framework/issues/29758',
165+
});
166+
167+
await page.setContent(
168+
`
169+
<ion-item>
170+
<ion-checkbox>
171+
Checkbox
172+
</ion-checkbox>
173+
</ion-item>
174+
`,
175+
config
176+
);
177+
178+
const item = page.locator('ion-item');
179+
const onClick = await page.spyOnEvent('click');
180+
181+
// Click the padding area (5px from left edge)
182+
await item.click({
183+
position: {
184+
x: 5,
185+
y: 5,
186+
},
187+
});
188+
189+
expect(onClick).toHaveReceivedEventTimes(1);
190+
191+
// Verify that the event target is the checkbox and not the item
192+
const event = onClick.events[0];
193+
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-checkbox');
194+
});
195+
});
196+
});

‎core/src/components/input/input.scss

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,10 @@
107107

108108
width: 100%;
109109
max-width: 100%;
110+
111+
// Ensure the input fills the full height of the native wrapper.
112+
// This prevents the wrapper from being the click event target.
113+
height: 100%;
110114
max-height: 100%;
111115

112116
border: 0;

‎core/src/components/input/input.tsx

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,18 @@
11
import type { ComponentInterface, EventEmitter } from '@stencil/core';
2-
import { Build, Component, Element, Event, Host, Method, Prop, State, Watch, forceUpdate, h } from '@stencil/core';
2+
import {
3+
Build,
4+
Component,
5+
Element,
6+
Event,
7+
Host,
8+
Listen,
9+
Method,
10+
Prop,
11+
State,
12+
Watch,
13+
forceUpdate,
14+
h,
15+
} from '@stencil/core';
316
import type { NotchController } from '@utils/forms';
417
import { createNotchController } from '@utils/forms';
518
import type { Attributes } from '@utils/helpers';
@@ -363,6 +376,19 @@ export class Input implements ComponentInterface {
363376
forceUpdate(this);
364377
}
365378

379+
/**
380+
* This prevents the native input from emitting the click event.
381+
* Instead, the click event from the ion-input is emitted.
382+
*/
383+
@Listen('click', { capture: true })
384+
onClickCapture(ev: Event) {
385+
const nativeInput = this.nativeInput;
386+
if (nativeInput && ev.target === nativeInput) {
387+
ev.stopPropagation();
388+
this.el.click();
389+
}
390+
}
391+
366392
componentWillLoad() {
367393
this.inheritedAttributes = {
368394
...inheritAriaAttributes(this.el),

‎core/src/components/input/test/item/input.e2e.ts

Lines changed: 89 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,11 @@ configs().forEach(({ title, screenshot, config }) => {
4949
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
5050
test.describe(title('input: item functionality'), () => {
5151
test('clicking padded space within item should focus the input', async ({ page }) => {
52+
test.info().annotations.push({
53+
type: 'issue',
54+
description: 'https://github.com/ionic-team/ionic-framework/issues/21982',
55+
});
56+
5257
await page.setContent(
5358
`
5459
<ion-item>
@@ -57,11 +62,12 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
5762
`,
5863
config
5964
);
60-
const itemNative = page.locator('.item-native');
65+
66+
const item = page.locator('ion-item');
6167
const input = page.locator('ion-input input');
6268

6369
// Clicks the padded space within the item
64-
await itemNative.click({
70+
await item.click({
6571
position: {
6672
x: 5,
6773
y: 5,
@@ -70,5 +76,86 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
7076

7177
await expect(input).toBeFocused();
7278
});
79+
80+
test('clicking padded space within item should fire one click event', async ({ page }) => {
81+
test.info().annotations.push({
82+
type: 'issue',
83+
description: 'https://github.com/ionic-team/ionic-framework/issues/29761',
84+
});
85+
86+
await page.setContent(
87+
`
88+
<ion-item>
89+
<ion-input label="Input"></ion-input>
90+
</ion-item>
91+
`,
92+
config
93+
);
94+
95+
const item = page.locator('ion-item');
96+
const onClick = await page.spyOnEvent('click');
97+
98+
// Click the padding area (5px from left edge)
99+
await item.click({
100+
position: {
101+
x: 5,
102+
y: 5,
103+
},
104+
});
105+
106+
expect(onClick).toHaveReceivedEventTimes(1);
107+
108+
// Verify that the event target is the input and not the item
109+
const event = onClick.events[0];
110+
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-input');
111+
});
112+
113+
test('clicking native wrapper should fire one click event', async ({ page }) => {
114+
await page.setContent(
115+
`
116+
<ion-item>
117+
<ion-input label="Input"></ion-input>
118+
</ion-item>
119+
`,
120+
config
121+
);
122+
123+
const nativeWrapper = page.locator('.native-wrapper');
124+
const onClick = await page.spyOnEvent('click');
125+
126+
await nativeWrapper.click({
127+
position: {
128+
x: 5,
129+
y: 5,
130+
},
131+
});
132+
133+
expect(onClick).toHaveReceivedEventTimes(1);
134+
135+
// Verify that the event target is the input and not the native wrapper
136+
const event = onClick.events[0];
137+
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-input');
138+
});
139+
140+
test('clicking native input within item should fire click event with target as ion-input', async ({ page }) => {
141+
await page.setContent(
142+
`
143+
<ion-item>
144+
<ion-input label="Input"></ion-input>
145+
</ion-item>
146+
`,
147+
config
148+
);
149+
150+
const nativeInput = page.locator('.native-input');
151+
const onClick = await page.spyOnEvent('click');
152+
153+
await nativeInput.click();
154+
expect(onClick).toHaveReceivedEventTimes(1);
155+
156+
// Verify that the event target is the ion-input and not the native input
157+
const event = onClick.events[0];
158+
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-input');
159+
});
73160
});
74161
});

‎core/src/components/item/item.tsx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
286286
if (firstInteractive !== undefined && !multipleInputs) {
287287
const path = ev.composedPath();
288288
const target = path[0] as HTMLElement;
289+
289290
if (ev.isTrusted) {
290291
/**
291292
* Dispatches a click event to the first interactive element,
@@ -304,9 +305,14 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
304305
*/
305306
if (firstInteractive.tagName === 'ION-INPUT' || firstInteractive.tagName === 'ION-TEXTAREA') {
306307
(firstInteractive as HTMLIonInputElement | HTMLIonTextareaElement).setFocus();
307-
} else {
308-
firstInteractive.click();
309308
}
309+
firstInteractive.click();
310+
/**
311+
* Stop the item event from being triggered
312+
* as the firstInteractive click event will also
313+
* trigger the item click event.
314+
*/
315+
ev.stopImmediatePropagation();
310316
}
311317
}
312318
}

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

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,16 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, co
7878
await expect(list).toHaveScreenshot(screenshot(`radio-stacked-label-in-item`));
7979
});
8080
});
81+
});
8182

82-
test.describe(title('radio: ionChange'), () => {
83+
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
84+
test.describe(title('radio: item functionality'), () => {
8385
test('clicking padded space within item should click the radio', async ({ page }) => {
86+
test.info().annotations.push({
87+
type: 'issue',
88+
description: 'https://github.com/ionic-team/ionic-framework/issues/27169',
89+
});
90+
8491
await page.setContent(
8592
`
8693
<ion-radio-group>
@@ -93,11 +100,11 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, co
93100
`,
94101
config
95102
);
96-
const itemNative = page.locator('.item-native');
103+
const item = page.locator('ion-item');
97104
const ionChange = await page.spyOnEvent('ionChange');
98105

99106
// Clicks the padded space within the item
100-
await itemNative.click({
107+
await item.click({
101108
position: {
102109
x: 5,
103110
y: 5,
@@ -106,5 +113,40 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, co
106113

107114
expect(ionChange).toHaveReceivedEvent();
108115
});
116+
117+
test('clicking padded space within item should fire one click event', async ({ page }) => {
118+
test.info().annotations.push({
119+
type: 'issue',
120+
description: 'https://github.com/ionic-team/ionic-framework/issues/29758',
121+
});
122+
123+
await page.setContent(
124+
`
125+
<ion-item>
126+
<ion-radio>
127+
Radio
128+
</ion-radio>
129+
</ion-item>
130+
`,
131+
config
132+
);
133+
134+
const item = page.locator('ion-item');
135+
const onClick = await page.spyOnEvent('click');
136+
137+
// Click the padding area (5px from left edge)
138+
await item.click({
139+
position: {
140+
x: 5,
141+
y: 5,
142+
},
143+
});
144+
145+
expect(onClick).toHaveReceivedEventTimes(1);
146+
147+
// Verify that the event target is the radio and not the item
148+
const event = onClick.events[0];
149+
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-radio');
150+
});
109151
});
110152
});

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

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -294,34 +294,6 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
294294
await select.evaluate((el: HTMLIonSelectElement) => (el.value = 'banana'));
295295
await expect(ionChange).not.toHaveReceivedEvent();
296296
});
297-
298-
test('clicking padded space within item should click the select', async ({ page }) => {
299-
await page.setContent(
300-
`
301-
<ion-item>
302-
<ion-select label="Fruit" interface="action-sheet">
303-
<ion-select-option value="apple">Apple</ion-select-option>
304-
<ion-select-option value="banana">Banana</ion-select-option>
305-
</ion-select>
306-
</ion-item>
307-
`,
308-
config
309-
);
310-
const itemNative = page.locator('.item-native');
311-
const ionActionSheetDidPresent = await page.spyOnEvent('ionActionSheetDidPresent');
312-
313-
// Clicks the padded space within the item
314-
await itemNative.click({
315-
position: {
316-
x: 5,
317-
y: 5,
318-
},
319-
});
320-
321-
await ionActionSheetDidPresent.next();
322-
323-
expect(ionActionSheetDidPresent).toHaveReceivedEvent();
324-
});
325297
});
326298
});
327299

0 commit comments

Comments
 (0)