Skip to content

Commit 4f5cd5e

Browse files
committed
fix(inputs): fixing issue on several input types (checkbox, select, textarea, toggle, input) where clicking on a label would trigger onclick twice
1 parent 8ef79cf commit 4f5cd5e

File tree

10 files changed

+221
-4
lines changed

10 files changed

+221
-4
lines changed

core/src/components/checkbox/checkbox.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,14 @@ export class Checkbox implements ComponentInterface {
199199
this.toggleChecked(ev);
200200
};
201201

202+
/**
203+
* Stops propagation when the display label is clicked,
204+
* otherwise, two clicks will be triggered.
205+
*/
206+
private onDivLabelClick = (ev: MouseEvent) => {
207+
ev.stopPropagation();
208+
}
209+
202210
private getHintTextID(): string | undefined {
203211
const { el, helperText, errorText, helperTextId, errorTextId } = this;
204212

@@ -314,6 +322,7 @@ export class Checkbox implements ComponentInterface {
314322
}}
315323
part="label"
316324
id={this.inputLabelId}
325+
onClick={this.onDivLabelClick}
317326
>
318327
<slot></slot>
319328
{this.renderHintText()}

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,4 +122,33 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
122122
expect(ionChange).toHaveReceivedEvent();
123123
});
124124
});
125+
126+
test.describe(title('checkbox: click'), () => {
127+
test('clicking a checkbox label should only trigger onclick once', async ({ page }) => {
128+
// Create a spy function in page context
129+
await page.setContent(`<ion-checkbox onclick="console.log('click called')">Test Checkbox</ion-checkbox>`, config);
130+
131+
// Track calls to the exposed function
132+
let clickCount = 0;
133+
page.on('console', (msg) => {
134+
if (msg.text().includes('click called')) {
135+
clickCount++;
136+
}
137+
});
138+
139+
const input = page.locator('div.label-text-wrapper');
140+
141+
// Use position to make sure we click into the label enough to trigger
142+
// what would be the double click
143+
await input.click({
144+
position: {
145+
x: 5,
146+
y: 5,
147+
},
148+
});
149+
150+
// Verify the click was triggered exactly once
151+
expect(clickCount).toBe(1);
152+
});
153+
});
125154
});

core/src/components/input/input.tsx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -694,6 +694,18 @@ export class Input implements ComponentInterface {
694694
return this.label !== undefined || this.labelSlot !== null;
695695
}
696696

697+
/**
698+
* Stops propagation when the label is clicked,
699+
* otherwise, two clicks will be triggered.
700+
*/
701+
private onLabelClick = (ev: MouseEvent) => {
702+
// Only stop propagation if the click was directly on the label
703+
// and not on the input or other child elements
704+
if (ev.target === ev.currentTarget) {
705+
ev.stopPropagation();
706+
}
707+
}
708+
697709
/**
698710
* Renders the border container
699711
* when fill="outline".
@@ -789,7 +801,7 @@ export class Input implements ComponentInterface {
789801
* interactable, clicking the label would focus that instead
790802
* since it comes before the input in the DOM.
791803
*/}
792-
<label class="input-wrapper" htmlFor={inputId}>
804+
<label class="input-wrapper" htmlFor={inputId} onClick={this.onLabelClick}>
793805
{this.renderLabelContainer()}
794806
<div class="native-wrapper">
795807
<slot name="start"></slot>

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,4 +130,39 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, screenshot, c
130130
await expect(item).toHaveScreenshot(screenshot(`input-with-clear-button-item-color`));
131131
});
132132
});
133+
134+
test.describe(title('input: click'), () => {
135+
test('should trigger onclick only once when clicking the label', async ({ page }) => {
136+
// Create a spy function in page context
137+
await page.setContent(
138+
`
139+
<ion-input
140+
label="Click Me"
141+
value="Test Value"
142+
></ion-input>
143+
`,
144+
config
145+
);
146+
147+
// Track calls to the exposed function
148+
const clickEvent = await page.spyOnEvent('click');
149+
const input = page.locator('label.input-wrapper');
150+
151+
// Use position to make sure we click into the label enough to trigger
152+
// what would be the double click
153+
await input.click({
154+
position: {
155+
x: 5,
156+
y: 5,
157+
},
158+
});
159+
160+
// Verify the click was triggered exactly once
161+
expect(clickEvent).toHaveReceivedEventTimes(1);
162+
163+
// Verify that the event target is the checkbox and not the item
164+
const event = clickEvent.events[0];
165+
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('input');
166+
});
167+
});
133168
});

core/src/components/select/select.tsx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -911,6 +911,18 @@ export class Select implements ComponentInterface {
911911
return this.label !== undefined || this.labelSlot !== null;
912912
}
913913

914+
/**
915+
* Stops propagation when the label is clicked,
916+
* otherwise, two clicks will be triggered.
917+
*/
918+
private onLabelClick = (ev: MouseEvent) => {
919+
// Only stop propagation if the click was directly on the label
920+
// and not on the input or other child elements
921+
if (ev.target === ev.currentTarget) {
922+
ev.stopPropagation();
923+
}
924+
}
925+
914926
/**
915927
* Renders the border container
916928
* when fill="outline".
@@ -1173,7 +1185,7 @@ export class Select implements ComponentInterface {
11731185
[`select-label-placement-${labelPlacement}`]: true,
11741186
})}
11751187
>
1176-
<label class="select-wrapper" id="select-label">
1188+
<label class="select-wrapper" id="select-label" onClick={this.onLabelClick}>
11771189
{this.renderLabelContainer()}
11781190
<div class="select-wrapper-inner">
11791191
<slot name="start"></slot>

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

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { expect } from '@playwright/test';
2-
import { configs, test } from '@utils/test/playwright';
32
import type { E2ELocator } from '@utils/test/playwright';
3+
import { configs, test } from '@utils/test/playwright';
44

55
/**
66
* This checks that certain overlays open correctly. While the
@@ -150,6 +150,41 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
150150
expect(alerts.length).toBe(1);
151151
});
152152
});
153+
154+
test.describe(title('select: click'), () => {
155+
test('clicking a select label should only trigger onclick once', async ({ page }) => {
156+
// Create a spy function in page context
157+
await page.setContent(
158+
`
159+
<ion-select aria-label="Fruit" interface="alert">
160+
<ion-select-option value="apple">Apple</ion-select-option>
161+
<ion-select-option value="banana">Banana</ion-select-option>
162+
</ion-select>
163+
`,
164+
config
165+
);
166+
167+
// Track calls to the exposed function
168+
const clickEvent = await page.spyOnEvent('click');
169+
const input = page.locator('label.select-wrapper');
170+
171+
// Use position to make sure we click into the label enough to trigger
172+
// what would be the double click
173+
await input.click({
174+
position: {
175+
x: 5,
176+
y: 5,
177+
},
178+
});
179+
180+
// Verify the click was triggered exactly once
181+
expect(clickEvent).toHaveReceivedEventTimes(1);
182+
183+
// Verify that the event target is the checkbox and not the item
184+
const event = clickEvent.events[0];
185+
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-select');
186+
});
187+
});
153188
});
154189

155190
/**
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { expect } from '@playwright/test';
2+
import { configs, test } from '@utils/test/playwright';
3+
4+
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
5+
test.describe(title('textarea: click'), () => {
6+
test('should trigger onclick only once when clicking the label', async ({ page }) => {
7+
// Create a spy function in page context
8+
await page.setContent(`<ion-textarea label="Textarea"></ion-textarea>`, config);
9+
10+
// Track calls to the exposed function
11+
const clickEvent = await page.spyOnEvent('click');
12+
const input = page.locator('label.textarea-wrapper');
13+
14+
// Use position to make sure we click into the label enough to trigger
15+
// what would be the double click
16+
await input.click({
17+
position: {
18+
x: 5,
19+
y: 5,
20+
},
21+
});
22+
23+
// Verify the click was triggered exactly once
24+
expect(clickEvent).toHaveReceivedEventTimes(1);
25+
26+
// Verify that the event target is the checkbox and not the item
27+
const event = clickEvent.events[0];
28+
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('textarea');
29+
});
30+
});
31+
});

core/src/components/textarea/textarea.tsx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -558,6 +558,18 @@ export class Textarea implements ComponentInterface {
558558
return this.label !== undefined || this.labelSlot !== null;
559559
}
560560

561+
/**
562+
* Stops propagation when the label is clicked,
563+
* otherwise, two clicks will be triggered.
564+
*/
565+
private onLabelClick = (ev: MouseEvent) => {
566+
// Only stop propagation if the click was directly on the label
567+
// and not on the input or other child elements
568+
if (ev.target === ev.currentTarget) {
569+
ev.stopPropagation();
570+
}
571+
}
572+
561573
/**
562574
* Renders the border container when fill="outline".
563575
*/
@@ -712,7 +724,7 @@ export class Textarea implements ComponentInterface {
712724
* interactable, clicking the label would focus that instead
713725
* since it comes before the textarea in the DOM.
714726
*/}
715-
<label class="textarea-wrapper" htmlFor={inputId}>
727+
<label class="textarea-wrapper" htmlFor={inputId} onClick={this.onLabelClick}>
716728
{this.renderLabelContainer()}
717729
<div class="textarea-wrapper-inner">
718730
{/**
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { expect } from '@playwright/test';
2+
import { configs, test } from '@utils/test/playwright';
3+
4+
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
5+
test.describe(title('toggle: click'), () => {
6+
test('should trigger onclick only once when clicking the label', async ({ page }) => {
7+
// Create a spy function in page context
8+
await page.setContent(`<ion-toggle onclick="console.log('click called')">my label</ion-toggle>`, config);
9+
10+
// Track calls to the exposed function
11+
let clickCount = 0;
12+
page.on('console', (msg) => {
13+
if (msg.text().includes('click called')) {
14+
clickCount++;
15+
}
16+
});
17+
18+
const input = page.locator('div.label-text-wrapper');
19+
20+
// Use position to make sure we click into the label enough to trigger
21+
// what would be the double click
22+
await input.click({
23+
position: {
24+
x: 5,
25+
y: 5,
26+
},
27+
});
28+
29+
// Verify the click was triggered exactly once
30+
expect(clickCount).toBe(1);
31+
});
32+
});
33+
});

core/src/components/toggle/toggle.tsx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,14 @@ export class Toggle implements ComponentInterface {
268268
}
269269
};
270270

271+
/**
272+
* Stops propagation when the display label is clicked,
273+
* otherwise, two clicks will be triggered.
274+
*/
275+
private onDivLabelClick = (ev: MouseEvent) => {
276+
ev.stopPropagation();
277+
}
278+
271279
private onFocus = () => {
272280
this.ionFocus.emit();
273281
};
@@ -437,6 +445,7 @@ export class Toggle implements ComponentInterface {
437445
}}
438446
part="label"
439447
id={inputLabelId}
448+
onClick={this.onDivLabelClick}
440449
>
441450
<slot></slot>
442451
{this.renderHintText()}

0 commit comments

Comments
 (0)