Skip to content

fix(item): emit click event once when clicking padded space on item and emit correct element #30373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
23 changes: 0 additions & 23 deletions core/src/components/checkbox/test/basic/checkbox.e2e.ts
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was moved to checkbox/test/item to match the other components.

Original file line number Diff line number Diff line change
Expand Up @@ -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(
`
<ion-item>
<ion-checkbox>Size</ion-checkbox>
</ion-item>
`,
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();
});
});
});
67 changes: 67 additions & 0 deletions core/src/components/checkbox/test/item/checkbox.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
`
<ion-item>
<ion-checkbox>Size</ion-checkbox>
</ion-item>
`,
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(
`
<ion-item>
<ion-checkbox>
Checkbox
</ion-checkbox>
</ion-item>
`,
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');
});
});
});
10 changes: 10 additions & 0 deletions core/src/components/checkbox/test/item/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,16 @@ <h1>Multiline Label</h1>
</div>
</div>
</ion-content>

<script>
const items = document.querySelectorAll('ion-item');

for (var i = 0; i < items.length; i++) {
items[i].addEventListener('click', function (ev) {
console.log('item clicked', ev.target);
});
}
</script>
</ion-app>
</body>
</html>
4 changes: 4 additions & 0 deletions core/src/components/input/input.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
28 changes: 27 additions & 1 deletion core/src/components/input/input.tsx
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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),
Expand Down
10 changes: 10 additions & 0 deletions core/src/components/input/test/item/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,15 @@ <h2>Inset List</h2>
</div>
</ion-content>
</ion-app>

<script>
const items = document.querySelectorAll('ion-item');

for (var i = 0; i < items.length; i++) {
items[i].addEventListener('click', function (ev) {
console.log('item clicked', ev.target);
});
}
</script>
</body>
</html>
91 changes: 89 additions & 2 deletions core/src/components/input/test/item/input.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
`
<ion-item>
Expand All @@ -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,
Expand All @@ -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(
`
<ion-item>
<ion-input label="Input"></ion-input>
</ion-item>
`,
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(
`
<ion-item>
<ion-input label="Input"></ion-input>
</ion-item>
`,
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(
`
<ion-item>
<ion-input label="Input"></ion-input>
</ion-item>
`,
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');
});
});
});
10 changes: 8 additions & 2 deletions core/src/components/item/item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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();
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions core/src/components/radio/test/item/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -207,5 +207,15 @@ <h1>Multiline Label</h1>
</div>
</ion-content>
</ion-app>

<script>
const items = document.querySelectorAll('ion-item');

for (var i = 0; i < items.length; i++) {
items[i].addEventListener('click', function (ev) {
console.log('item clicked', ev.target);
});
}
</script>
</body>
</html>
Loading
Loading