Skip to content

fix(labels): prevent clicking a label from triggering onClick twice on several components #30384

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

Merged
merged 13 commits into from
May 2, 2025
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions core/src/components/checkbox/checkbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,14 @@ export class Checkbox implements ComponentInterface {
this.toggleChecked(ev);
};

/**
* Stops propagation when the display label is clicked,
* otherwise, two clicks will be triggered.
*/
private onDivLabelClick = (ev: MouseEvent) => {
ev.stopPropagation();
};

private getHintTextID(): string | undefined {
const { el, helperText, errorText, helperTextId, errorTextId } = this;

Expand Down Expand Up @@ -314,6 +322,7 @@ export class Checkbox implements ComponentInterface {
}}
part="label"
id={this.inputLabelId}
onClick={this.onDivLabelClick}
>
<slot></slot>
{this.renderHintText()}
Expand Down
29 changes: 29 additions & 0 deletions core/src/components/checkbox/test/basic/checkbox.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,33 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
expect(ionChange).not.toHaveReceivedEvent();
});
});

test.describe(title('checkbox: click'), () => {
test('clicking a checkbox label should only trigger onclick once', async ({ page }) => {
// Create a spy function in page context
await page.setContent(`<ion-checkbox onclick="console.log('click called')">Test Checkbox</ion-checkbox>`, config);

// Track calls to the exposed function
let clickCount = 0;
page.on('console', (msg) => {
if (msg.text().includes('click called')) {
clickCount++;
}
});

const input = page.locator('div.label-text-wrapper');

// Use position to make sure we click into the label enough to trigger
// what would be the double click
await input.click({
position: {
x: 5,
y: 5,
},
});

// Verify the click was triggered exactly once
expect(clickCount).toBe(1);
});
});
});
16 changes: 14 additions & 2 deletions core/src/components/input/input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,18 @@ export class Input implements ComponentInterface {
return this.label !== undefined || this.labelSlot !== null;
}

/**
* Stops propagation when the label is clicked,
* otherwise, two clicks will be triggered.
*/
private onLabelClick = (ev: MouseEvent) => {
// Only stop propagation if the click was directly on the label
// and not on the input or other child elements
if (ev.target === ev.currentTarget) {
ev.stopPropagation();
}
};

/**
* Renders the border container
* when fill="outline".
Expand Down Expand Up @@ -815,9 +827,9 @@ export class Input implements ComponentInterface {
* interactable, clicking the label would focus that instead
* since it comes before the input in the DOM.
*/}
<label class="input-wrapper" htmlFor={inputId}>
<label class="input-wrapper" htmlFor={inputId} onClick={this.onLabelClick}>
{this.renderLabelContainer()}
<div class="native-wrapper">
<div class="native-wrapper" onClick={this.onLabelClick}>
<slot name="start"></slot>
<input
class="native-input"
Expand Down
77 changes: 77 additions & 0 deletions core/src/components/input/test/basic/input.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,4 +130,81 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, screenshot, c
await expect(item).toHaveScreenshot(screenshot(`input-with-clear-button-item-color`));
});
});

test.describe(title('input: click'), () => {
test('should trigger onclick only once when clicking the label', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/30165',
});
// Create a spy function in page context
await page.setContent(
`
<ion-input
label="Click Me"
value="Test Value"
></ion-input>
`,
config
);

// Track calls to the exposed function
const clickEvent = await page.spyOnEvent('click');
const input = page.locator('label.input-wrapper');

// Use position to make sure we click into the label enough to trigger
// what would be the double click
await input.click({
position: {
x: 5,
y: 5,
},
});

// Verify the click was triggered exactly once
expect(clickEvent).toHaveReceivedEventTimes(1);

// Verify that the event target is the checkbox and not the item
const event = clickEvent.events[0];
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-input');
});

test('should trigger onclick only once when clicking the wrapper', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/30165',
});
// Create a spy function in page context
await page.setContent(
`
<ion-input
label="Click Me"
value="Test Value"
label-placement="floating"
></ion-input>
`,
config
);

// Track calls to the exposed function
const clickEvent = await page.spyOnEvent('click');
const input = page.locator('div.native-wrapper');

// Use position to make sure we click into the label enough to trigger
// what would be the double click
await input.click({
position: {
x: 1,
y: 1,
},
});

// Verify the click was triggered exactly once
expect(clickEvent).toHaveReceivedEventTimes(1);

// Verify that the event target is the checkbox and not the item
const event = clickEvent.events[0];
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-input');
});
});
});
14 changes: 13 additions & 1 deletion core/src/components/select/select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -911,6 +911,18 @@ export class Select implements ComponentInterface {
return this.label !== undefined || this.labelSlot !== null;
}

/**
* Stops propagation when the label is clicked,
* otherwise, two clicks will be triggered.
*/
private onLabelClick = (ev: MouseEvent) => {
// Only stop propagation if the click was directly on the label
// and not on the input or other child elements
if (ev.target === ev.currentTarget) {
ev.stopPropagation();
}
};

/**
* Renders the border container
* when fill="outline".
Expand Down Expand Up @@ -1173,7 +1185,7 @@ export class Select implements ComponentInterface {
[`select-label-placement-${labelPlacement}`]: true,
})}
>
<label class="select-wrapper" id="select-label">
<label class="select-wrapper" id="select-label" onClick={this.onLabelClick}>
{this.renderLabelContainer()}
<div class="select-wrapper-inner">
<slot name="start"></slot>
Expand Down
41 changes: 40 additions & 1 deletion core/src/components/select/test/basic/select.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';
import type { E2ELocator } from '@utils/test/playwright';
import { configs, test } from '@utils/test/playwright';

/**
* This checks that certain overlays open correctly. While the
Expand Down Expand Up @@ -150,6 +150,45 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
expect(alerts.length).toBe(1);
});
});

test.describe(title('select: click'), () => {
test('clicking a select label should only trigger onclick once', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/30165',
});
// Create a spy function in page context
await page.setContent(
`
<ion-select aria-label="Fruit" interface="alert">
<ion-select-option value="apple">Apple</ion-select-option>
<ion-select-option value="banana">Banana</ion-select-option>
</ion-select>
`,
config
);

// Track calls to the exposed function
const clickEvent = await page.spyOnEvent('click');
const input = page.locator('label.select-wrapper');

// Use position to make sure we click into the label enough to trigger
// what would be the double click
await input.click({
position: {
x: 5,
y: 5,
},
});

// Verify the click was triggered exactly once
expect(clickEvent).toHaveReceivedEventTimes(1);

// Verify that the event target is the checkbox and not the item
const event = clickEvent.events[0];
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-select');
});
});
});

/**
Expand Down
35 changes: 35 additions & 0 deletions core/src/components/textarea/test/basic/textarea.e2e.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';

configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
test.describe(title('textarea: click'), () => {
test('should trigger onclick only once when clicking the label', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/30165',
});
// Create a spy function in page context
await page.setContent(`<ion-textarea label="Textarea"></ion-textarea>`, config);

// Track calls to the exposed function
const clickEvent = await page.spyOnEvent('click');
const input = page.locator('label.textarea-wrapper');

// Use position to make sure we click into the label enough to trigger
// what would be the double click
await input.click({
position: {
x: 5,
y: 5,
},
});

// Verify the click was triggered exactly once
expect(clickEvent).toHaveReceivedEventTimes(1);

// Verify that the event target is the checkbox and not the item
const event = clickEvent.events[0];
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-textarea');
});
});
});
14 changes: 13 additions & 1 deletion core/src/components/textarea/textarea.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,18 @@ export class Textarea implements ComponentInterface {
return this.label !== undefined || this.labelSlot !== null;
}

/**
* Stops propagation when the label is clicked,
* otherwise, two clicks will be triggered.
*/
private onLabelClick = (ev: MouseEvent) => {
// Only stop propagation if the click was directly on the label
// and not on the input or other child elements
if (ev.target === ev.currentTarget) {
ev.stopPropagation();
}
};

/**
* Renders the border container when fill="outline".
*/
Expand Down Expand Up @@ -726,7 +738,7 @@ export class Textarea implements ComponentInterface {
* interactable, clicking the label would focus that instead
* since it comes before the textarea in the DOM.
*/}
<label class="textarea-wrapper" htmlFor={inputId}>
<label class="textarea-wrapper" htmlFor={inputId} onClick={this.onLabelClick}>
{this.renderLabelContainer()}
<div class="textarea-wrapper-inner">
{/**
Expand Down
33 changes: 33 additions & 0 deletions core/src/components/toggle/test/basic/toggle.e2e.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';

configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
test.describe(title('toggle: click'), () => {
test('should trigger onclick only once when clicking the label', async ({ page }) => {
// Create a spy function in page context
await page.setContent(`<ion-toggle onclick="console.log('click called')">my label</ion-toggle>`, config);

// Track calls to the exposed function
let clickCount = 0;
page.on('console', (msg) => {
if (msg.text().includes('click called')) {
clickCount++;
}
});

const input = page.locator('div.label-text-wrapper');

// Use position to make sure we click into the label enough to trigger
// what would be the double click
await input.click({
position: {
x: 5,
y: 5,
},
});

// Verify the click was triggered exactly once
expect(clickCount).toBe(1);
});
});
});
9 changes: 9 additions & 0 deletions core/src/components/toggle/toggle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,14 @@ export class Toggle implements ComponentInterface {
}
};

/**
* Stops propagation when the display label is clicked,
* otherwise, two clicks will be triggered.
*/
private onDivLabelClick = (ev: MouseEvent) => {
ev.stopPropagation();
};

private onFocus = () => {
this.ionFocus.emit();
};
Expand Down Expand Up @@ -437,6 +445,7 @@ export class Toggle implements ComponentInterface {
}}
part="label"
id={inputLabelId}
onClick={this.onDivLabelClick}
>
<slot></slot>
{this.renderHintText()}
Expand Down
Loading