From 7a9d138e3d5ecde55c12ff337ca29052a9194d69 Mon Sep 17 00:00:00 2001 From: Brandy Smith Date: Wed, 30 Apr 2025 13:12:54 -0400 Subject: [PATCH] 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 <6577830+brandyscarney@users.noreply.github.com> --- .../checkbox/test/basic/checkbox.e2e.ts | 23 ----- .../checkbox/test/item/checkbox.e2e.ts | 67 ++++++++++++++ core/src/components/input/input.scss | 4 + core/src/components/input/input.tsx | 28 +++++- .../components/input/test/item/input.e2e.ts | 91 ++++++++++++++++++- core/src/components/item/item.tsx | 10 +- .../components/radio/test/item/radio.e2e.ts | 48 +++++++++- .../select/test/basic/select.e2e.ts | 28 ------ .../components/select/test/item/select.e2e.ts | 75 +++++++++++++++ .../textarea/test/item/textarea.e2e.ts | 65 ++++++++++++- core/src/components/textarea/textarea.tsx | 14 +++ .../components/toggle/test/item/toggle.e2e.ts | 48 +++++++++- 12 files changed, 437 insertions(+), 64 deletions(-) diff --git a/core/src/components/checkbox/test/basic/checkbox.e2e.ts b/core/src/components/checkbox/test/basic/checkbox.e2e.ts index 957c6da31b..1a41b33959 100644 --- a/core/src/components/checkbox/test/basic/checkbox.e2e.ts +++ b/core/src/components/checkbox/test/basic/checkbox.e2e.ts @@ -98,28 +98,5 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => await checkbox.evaluate((el: HTMLIonCheckboxElement) => (el.checked = true)); expect(ionChange).not.toHaveReceivedEvent(); }); - - test('clicking padded space within item should click the checkbox', async ({ page }) => { - await page.setContent( - ` - - Size - - `, - config - ); - const itemNative = page.locator('.item-native'); - const ionChange = await page.spyOnEvent('ionChange'); - - // Clicks the padded space within the item - await itemNative.click({ - position: { - x: 5, - y: 5, - }, - }); - - expect(ionChange).toHaveReceivedEvent(); - }); }); }); diff --git a/core/src/components/checkbox/test/item/checkbox.e2e.ts b/core/src/components/checkbox/test/item/checkbox.e2e.ts index bdcf1892da..341839739b 100644 --- a/core/src/components/checkbox/test/item/checkbox.e2e.ts +++ b/core/src/components/checkbox/test/item/checkbox.e2e.ts @@ -127,3 +127,70 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { }); }); }); + +configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('checkbox: item functionality'), () => { + test('clicking padded space within item should click the checkbox', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/27169', + }); + + await page.setContent( + ` + + Size + + `, + config + ); + const item = page.locator('ion-item'); + const ionChange = await page.spyOnEvent('ionChange'); + + // Clicks the padded space within the item + await item.click({ + position: { + x: 5, + y: 5, + }, + }); + + expect(ionChange).toHaveReceivedEvent(); + }); + + test('clicking padded space within item should fire one click event', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/29758', + }); + + await page.setContent( + ` + + + Checkbox + + + `, + config + ); + + const item = page.locator('ion-item'); + const onClick = await page.spyOnEvent('click'); + + // Click the padding area (5px from left edge) + await item.click({ + position: { + x: 5, + y: 5, + }, + }); + + expect(onClick).toHaveReceivedEventTimes(1); + + // Verify that the event target is the checkbox and not the item + const event = onClick.events[0]; + expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-checkbox'); + }); + }); +}); diff --git a/core/src/components/input/input.scss b/core/src/components/input/input.scss index cc648e6e2f..57d438eb2d 100644 --- a/core/src/components/input/input.scss +++ b/core/src/components/input/input.scss @@ -107,6 +107,10 @@ width: 100%; max-width: 100%; + + // Ensure the input fills the full height of the native wrapper. + // This prevents the wrapper from being the click event target. + height: 100%; max-height: 100%; border: 0; diff --git a/core/src/components/input/input.tsx b/core/src/components/input/input.tsx index 42bee8e005..5f4b125714 100644 --- a/core/src/components/input/input.tsx +++ b/core/src/components/input/input.tsx @@ -1,5 +1,18 @@ import type { ComponentInterface, EventEmitter } from '@stencil/core'; -import { Build, Component, Element, Event, Host, Method, Prop, State, Watch, forceUpdate, h } from '@stencil/core'; +import { + Build, + Component, + Element, + Event, + Host, + Listen, + Method, + Prop, + State, + Watch, + forceUpdate, + h, +} from '@stencil/core'; import type { NotchController } from '@utils/forms'; import { createNotchController } from '@utils/forms'; import type { Attributes } from '@utils/helpers'; @@ -363,6 +376,19 @@ export class Input implements ComponentInterface { forceUpdate(this); } + /** + * This prevents the native input from emitting the click event. + * Instead, the click event from the ion-input is emitted. + */ + @Listen('click', { capture: true }) + onClickCapture(ev: Event) { + const nativeInput = this.nativeInput; + if (nativeInput && ev.target === nativeInput) { + ev.stopPropagation(); + this.el.click(); + } + } + componentWillLoad() { this.inheritedAttributes = { ...inheritAriaAttributes(this.el), diff --git a/core/src/components/input/test/item/input.e2e.ts b/core/src/components/input/test/item/input.e2e.ts index 710e2f00b1..a4fd996c71 100644 --- a/core/src/components/input/test/item/input.e2e.ts +++ b/core/src/components/input/test/item/input.e2e.ts @@ -49,6 +49,11 @@ configs().forEach(({ title, screenshot, config }) => { configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => { test.describe(title('input: item functionality'), () => { test('clicking padded space within item should focus the input', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/21982', + }); + await page.setContent( ` @@ -57,11 +62,12 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => `, config ); - const itemNative = page.locator('.item-native'); + + const item = page.locator('ion-item'); const input = page.locator('ion-input input'); // Clicks the padded space within the item - await itemNative.click({ + await item.click({ position: { x: 5, y: 5, @@ -70,5 +76,86 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => await expect(input).toBeFocused(); }); + + test('clicking padded space within item should fire one click event', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/29761', + }); + + await page.setContent( + ` + + + + `, + config + ); + + const item = page.locator('ion-item'); + const onClick = await page.spyOnEvent('click'); + + // Click the padding area (5px from left edge) + await item.click({ + position: { + x: 5, + y: 5, + }, + }); + + expect(onClick).toHaveReceivedEventTimes(1); + + // Verify that the event target is the input and not the item + const event = onClick.events[0]; + expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-input'); + }); + + test('clicking native wrapper should fire one click event', async ({ page }) => { + await page.setContent( + ` + + + + `, + config + ); + + const nativeWrapper = page.locator('.native-wrapper'); + const onClick = await page.spyOnEvent('click'); + + await nativeWrapper.click({ + position: { + x: 5, + y: 5, + }, + }); + + expect(onClick).toHaveReceivedEventTimes(1); + + // Verify that the event target is the input and not the native wrapper + const event = onClick.events[0]; + expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-input'); + }); + + test('clicking native input within item should fire click event with target as ion-input', async ({ page }) => { + await page.setContent( + ` + + + + `, + config + ); + + const nativeInput = page.locator('.native-input'); + const onClick = await page.spyOnEvent('click'); + + await nativeInput.click(); + expect(onClick).toHaveReceivedEventTimes(1); + + // Verify that the event target is the ion-input and not the native input + const event = onClick.events[0]; + expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-input'); + }); }); }); diff --git a/core/src/components/item/item.tsx b/core/src/components/item/item.tsx index 6091e8b1cf..6384927de8 100644 --- a/core/src/components/item/item.tsx +++ b/core/src/components/item/item.tsx @@ -286,6 +286,7 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac if (firstInteractive !== undefined && !multipleInputs) { const path = ev.composedPath(); const target = path[0] as HTMLElement; + if (ev.isTrusted) { /** * Dispatches a click event to the first interactive element, @@ -304,9 +305,14 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac */ if (firstInteractive.tagName === 'ION-INPUT' || firstInteractive.tagName === 'ION-TEXTAREA') { (firstInteractive as HTMLIonInputElement | HTMLIonTextareaElement).setFocus(); - } else { - firstInteractive.click(); } + firstInteractive.click(); + /** + * Stop the item event from being triggered + * as the firstInteractive click event will also + * trigger the item click event. + */ + ev.stopImmediatePropagation(); } } } diff --git a/core/src/components/radio/test/item/radio.e2e.ts b/core/src/components/radio/test/item/radio.e2e.ts index 4fdf34bd4e..8c9fd4be95 100644 --- a/core/src/components/radio/test/item/radio.e2e.ts +++ b/core/src/components/radio/test/item/radio.e2e.ts @@ -78,9 +78,16 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, co await expect(list).toHaveScreenshot(screenshot(`radio-stacked-label-in-item`)); }); }); +}); - test.describe(title('radio: ionChange'), () => { +configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('radio: item functionality'), () => { test('clicking padded space within item should click the radio', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/27169', + }); + await page.setContent( ` @@ -93,11 +100,11 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, co `, config ); - const itemNative = page.locator('.item-native'); + const item = page.locator('ion-item'); const ionChange = await page.spyOnEvent('ionChange'); // Clicks the padded space within the item - await itemNative.click({ + await item.click({ position: { x: 5, y: 5, @@ -106,5 +113,40 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, co expect(ionChange).toHaveReceivedEvent(); }); + + test('clicking padded space within item should fire one click event', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/29758', + }); + + await page.setContent( + ` + + + Radio + + + `, + config + ); + + const item = page.locator('ion-item'); + const onClick = await page.spyOnEvent('click'); + + // Click the padding area (5px from left edge) + await item.click({ + position: { + x: 5, + y: 5, + }, + }); + + expect(onClick).toHaveReceivedEventTimes(1); + + // Verify that the event target is the radio and not the item + const event = onClick.events[0]; + expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-radio'); + }); }); }); diff --git a/core/src/components/select/test/basic/select.e2e.ts b/core/src/components/select/test/basic/select.e2e.ts index d5a9c3d220..63f1c8ca10 100644 --- a/core/src/components/select/test/basic/select.e2e.ts +++ b/core/src/components/select/test/basic/select.e2e.ts @@ -294,34 +294,6 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { await select.evaluate((el: HTMLIonSelectElement) => (el.value = 'banana')); await expect(ionChange).not.toHaveReceivedEvent(); }); - - test('clicking padded space within item should click the select', async ({ page }) => { - await page.setContent( - ` - - - Apple - Banana - - - `, - config - ); - const itemNative = page.locator('.item-native'); - const ionActionSheetDidPresent = await page.spyOnEvent('ionActionSheetDidPresent'); - - // Clicks the padded space within the item - await itemNative.click({ - position: { - x: 5, - y: 5, - }, - }); - - await ionActionSheetDidPresent.next(); - - expect(ionActionSheetDidPresent).toHaveReceivedEvent(); - }); }); }); diff --git a/core/src/components/select/test/item/select.e2e.ts b/core/src/components/select/test/item/select.e2e.ts index 05ca3e4abb..d0ad1d616d 100644 --- a/core/src/components/select/test/item/select.e2e.ts +++ b/core/src/components/select/test/item/select.e2e.ts @@ -61,3 +61,78 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => { }); }); }); + +configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('select: item functionality'), () => { + test('clicking padded space within item should click the select', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/27169', + }); + + await page.setContent( + ` + + + Apple + Banana + + + `, + config + ); + const item = page.locator('ion-item'); + const ionActionSheetDidPresent = await page.spyOnEvent('ionActionSheetDidPresent'); + + // Clicks the padded space within the item + await item.click({ + position: { + x: 5, + y: 5, + }, + }); + + await ionActionSheetDidPresent.next(); + + expect(ionActionSheetDidPresent).toHaveReceivedEvent(); + }); + + test('clicking padded space within item should fire one click event', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/29758', + }); + + await page.setContent( + ` + + + Apple + + + `, + config + ); + + const item = page.locator('ion-item'); + const onClick = await page.spyOnEvent('click'); + + // Click the padding area (5px from left edge) + await item.click({ + position: { + x: 5, + y: 5, + }, + }); + + expect(onClick).toHaveReceivedEventTimes(1); + + // Verify that the event target is the select and not the item + const event = onClick.events[0]; + expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-select'); + }); + }); +}); diff --git a/core/src/components/textarea/test/item/textarea.e2e.ts b/core/src/components/textarea/test/item/textarea.e2e.ts index ed5daeec44..9b3408733e 100644 --- a/core/src/components/textarea/test/item/textarea.e2e.ts +++ b/core/src/components/textarea/test/item/textarea.e2e.ts @@ -49,6 +49,11 @@ configs().forEach(({ title, screenshot, config }) => { configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => { test.describe(title('textarea: item functionality'), () => { test('clicking padded space within item should focus the textarea', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/21982', + }); + await page.setContent( ` @@ -57,11 +62,11 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => `, config ); - const itemNative = page.locator('.item-native'); + const item = page.locator('ion-item'); const textarea = page.locator('ion-textarea textarea'); // Clicks the padded space within the item - await itemNative.click({ + await item.click({ position: { x: 5, y: 5, @@ -70,5 +75,61 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => await expect(textarea).toBeFocused(); }); + + test('clicking padded space within item should fire one click event', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/29761', + }); + + await page.setContent( + ` + + + + `, + config + ); + + const item = page.locator('ion-item'); + const onClick = await page.spyOnEvent('click'); + + // Click the padding area (5px from left edge) + await item.click({ + position: { + x: 5, + y: 5, + }, + }); + + expect(onClick).toHaveReceivedEventTimes(1); + + // Verify that the event target is the input and not the item + const event = onClick.events[0]; + expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-textarea'); + }); + + test('clicking native textarea within item should fire click event with target as ion-textarea', async ({ + page, + }) => { + await page.setContent( + ` + + + + `, + config + ); + + const nativeTextarea = page.locator('.native-textarea'); + const onClick = await page.spyOnEvent('click'); + + await nativeTextarea.click(); + expect(onClick).toHaveReceivedEventTimes(1); + + // Verify that the event target is the ion-textarea and not the native textarea + const event = onClick.events[0]; + expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-textarea'); + }); }); }); diff --git a/core/src/components/textarea/textarea.tsx b/core/src/components/textarea/textarea.tsx index afb9e3bf6b..78541e9d66 100644 --- a/core/src/components/textarea/textarea.tsx +++ b/core/src/components/textarea/textarea.tsx @@ -5,6 +5,7 @@ import { Element, Event, Host, + Listen, Method, Prop, State, @@ -314,6 +315,19 @@ export class Textarea implements ComponentInterface { */ @Event() ionFocus!: EventEmitter; + /** + * This prevents the native input from emitting the click event. + * Instead, the click event from the ion-textarea is emitted. + */ + @Listen('click', { capture: true }) + onClickCapture(ev: Event) { + const nativeInput = this.nativeInput; + if (nativeInput && ev.target === nativeInput) { + ev.stopPropagation(); + this.el.click(); + } + } + connectedCallback() { const { el } = this; this.slotMutationController = createSlotMutationController(el, ['label', 'start', 'end'], () => forceUpdate(this)); diff --git a/core/src/components/toggle/test/item/toggle.e2e.ts b/core/src/components/toggle/test/item/toggle.e2e.ts index f5db0dab8a..866a382494 100644 --- a/core/src/components/toggle/test/item/toggle.e2e.ts +++ b/core/src/components/toggle/test/item/toggle.e2e.ts @@ -108,9 +108,16 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, co await expect(list).toHaveScreenshot(screenshot(`toggle-stacked-label-in-item`)); }); }); +}); - test.describe(title('toggle: ionChange'), () => { +configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('toggle: item functionality'), () => { test('clicking padded space within item should click the toggle', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/27169', + }); + await page.setContent( ` @@ -119,7 +126,7 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, co `, config ); - const itemNative = page.locator('.item-native'); + const item = page.locator('ion-item'); const ionChange = await page.spyOnEvent('ionChange'); /** @@ -132,7 +139,7 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, co * 2. iOS is inconsistent in their implementation and other controls can be activated by clicking the label. * 3. MD is consistent in their implementation and activates controls by clicking the label. */ - await itemNative.click({ + await item.click({ position: { x: 5, y: 5, @@ -141,5 +148,40 @@ configs({ directions: ['ltr'], modes: ['md'] }).forEach(({ title, screenshot, co expect(ionChange).toHaveReceivedEvent(); }); + + test('clicking padded space within item should fire one click event', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/29758', + }); + + await page.setContent( + ` + + + Toggle + + + `, + config + ); + + const item = page.locator('ion-item'); + const onClick = await page.spyOnEvent('click'); + + // Click the padding area (5px from left edge) + await item.click({ + position: { + x: 5, + y: 5, + }, + }); + + expect(onClick).toHaveReceivedEventTimes(1); + + // Verify that the event target is the toggle and not the item + const event = onClick.events[0]; + expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-toggle'); + }); }); });