Skip to content

Commit 7d639b0

Browse files
ShaneKthetaPCbrandyscarney
authored
fix(labels): prevent clicking a label from triggering onClick twice on several components (#30384)
Issue number: resolves #30165 --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> Currently, several components will trigger their `onClick` twice if you click on their labels. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> After this fix, the affected components will only trigger `onClick` once per click of their labels or click directly on the element. ## Does this introduce a breaking change? - [ ] Yes - [X] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> The affected components are: - Checkbox - Select - Textarea - Toggle - Input I also tested radio and range but could not reproduce the issue for them. Note that two of the components, checkbox and toggle, had to have special implementations for both their test and fix because of how the host component acts as the component for accessibility purposes. Current dev build: `8.5.7-dev.11746044124.147aab6c` --------- Co-authored-by: Maria Hutt <thetaPC@users.noreply.github.com> Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
1 parent bf396c9 commit 7d639b0

File tree

10 files changed

+282
-5
lines changed

10 files changed

+282
-5
lines changed

‎core/src/components/checkbox/checkbox.tsx

+9
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

+34
Original file line numberDiff line numberDiff line change
@@ -99,4 +99,38 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
9999
expect(ionChange).not.toHaveReceivedEvent();
100100
});
101101
});
102+
103+
test.describe(title('checkbox: click'), () => {
104+
test('should trigger onclick only once when clicking the label', async ({ page }, testInfo) => {
105+
testInfo.annotations.push({
106+
type: 'issue',
107+
description: 'https://github.com/ionic-team/ionic-framework/issues/30165',
108+
});
109+
110+
// Create a spy function in page context
111+
await page.setContent(`<ion-checkbox onclick="console.log('click called')">Test Checkbox</ion-checkbox>`, config);
112+
113+
// Track calls to the exposed function
114+
let clickCount = 0;
115+
page.on('console', (msg) => {
116+
if (msg.text().includes('click called')) {
117+
clickCount++;
118+
}
119+
});
120+
121+
const input = page.locator('div.label-text-wrapper');
122+
123+
// Use position to make sure we click into the label enough to trigger
124+
// what would be the double click
125+
await input.click({
126+
position: {
127+
x: 5,
128+
y: 5,
129+
},
130+
});
131+
132+
// Verify the click was triggered exactly once
133+
expect(clickCount).toBe(1);
134+
});
135+
});
102136
});

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

+14-2
Original file line numberDiff line numberDiff line change
@@ -720,6 +720,18 @@ export class Input implements ComponentInterface {
720720
return this.label !== undefined || this.labelSlot !== null;
721721
}
722722

723+
/**
724+
* Stops propagation when the label is clicked,
725+
* otherwise, two clicks will be triggered.
726+
*/
727+
private onLabelClick = (ev: MouseEvent) => {
728+
// Only stop propagation if the click was directly on the label
729+
// and not on the input or other child elements
730+
if (ev.target === ev.currentTarget) {
731+
ev.stopPropagation();
732+
}
733+
};
734+
723735
/**
724736
* Renders the border container
725737
* when fill="outline".
@@ -815,9 +827,9 @@ export class Input implements ComponentInterface {
815827
* interactable, clicking the label would focus that instead
816828
* since it comes before the input in the DOM.
817829
*/}
818-
<label class="input-wrapper" htmlFor={inputId}>
830+
<label class="input-wrapper" htmlFor={inputId} onClick={this.onLabelClick}>
819831
{this.renderLabelContainer()}
820-
<div class="native-wrapper">
832+
<div class="native-wrapper" onClick={this.onLabelClick}>
821833
<slot name="start"></slot>
822834
<input
823835
class="native-input"

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

+77
Original file line numberDiff line numberDiff line change
@@ -130,4 +130,81 @@ 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 }, testInfo) => {
136+
testInfo.annotations.push({
137+
type: 'issue',
138+
description: 'https://github.com/ionic-team/ionic-framework/issues/30165',
139+
});
140+
// Create a spy function in page context
141+
await page.setContent(
142+
`
143+
<ion-input
144+
label="Click Me"
145+
value="Test Value"
146+
></ion-input>
147+
`,
148+
config
149+
);
150+
151+
// Track calls to the exposed function
152+
const clickEvent = await page.spyOnEvent('click');
153+
const input = page.locator('label.input-wrapper');
154+
155+
// Use position to make sure we click into the label enough to trigger
156+
// what would be the double click
157+
await input.click({
158+
position: {
159+
x: 5,
160+
y: 5,
161+
},
162+
});
163+
164+
// Verify the click was triggered exactly once
165+
expect(clickEvent).toHaveReceivedEventTimes(1);
166+
167+
// Verify that the event target is the checkbox and not the item
168+
const event = clickEvent.events[0];
169+
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-input');
170+
});
171+
172+
test('should trigger onclick only once when clicking the wrapper', async ({ page }, testInfo) => {
173+
testInfo.annotations.push({
174+
type: 'issue',
175+
description: 'https://github.com/ionic-team/ionic-framework/issues/30165',
176+
});
177+
// Create a spy function in page context
178+
await page.setContent(
179+
`
180+
<ion-input
181+
label="Click Me"
182+
value="Test Value"
183+
label-placement="floating"
184+
></ion-input>
185+
`,
186+
config
187+
);
188+
189+
// Track calls to the exposed function
190+
const clickEvent = await page.spyOnEvent('click');
191+
const input = page.locator('div.native-wrapper');
192+
193+
// Use position to make sure we click into the label enough to trigger
194+
// what would be the double click
195+
await input.click({
196+
position: {
197+
x: 1,
198+
y: 1,
199+
},
200+
});
201+
202+
// Verify the click was triggered exactly once
203+
expect(clickEvent).toHaveReceivedEventTimes(1);
204+
205+
// Verify that the event target is the checkbox and not the item
206+
const event = clickEvent.events[0];
207+
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-input');
208+
});
209+
});
133210
});

‎core/src/components/select/select.tsx

+13-1
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

+40-1
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,45 @@ 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('should trigger onclick only once when clicking the label', async ({ page }, testInfo) => {
156+
testInfo.annotations.push({
157+
type: 'issue',
158+
description: 'https://github.com/ionic-team/ionic-framework/issues/30165',
159+
});
160+
// Create a spy function in page context
161+
await page.setContent(
162+
`
163+
<ion-select aria-label="Fruit" interface="alert">
164+
<ion-select-option value="apple">Apple</ion-select-option>
165+
<ion-select-option value="banana">Banana</ion-select-option>
166+
</ion-select>
167+
`,
168+
config
169+
);
170+
171+
// Track calls to the exposed function
172+
const clickEvent = await page.spyOnEvent('click');
173+
const input = page.locator('label.select-wrapper');
174+
175+
// Use position to make sure we click into the label enough to trigger
176+
// what would be the double click
177+
await input.click({
178+
position: {
179+
x: 5,
180+
y: 5,
181+
},
182+
});
183+
184+
// Verify the click was triggered exactly once
185+
expect(clickEvent).toHaveReceivedEventTimes(1);
186+
187+
// Verify that the event target is the checkbox and not the item
188+
const event = clickEvent.events[0];
189+
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-select');
190+
});
191+
});
153192
});
154193

155194
/**

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

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
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 }, testInfo) => {
7+
testInfo.annotations.push({
8+
type: 'issue',
9+
description: 'https://github.com/ionic-team/ionic-framework/issues/30165',
10+
});
11+
// Create a spy function in page context
12+
await page.setContent(`<ion-textarea label="Textarea"></ion-textarea>`, config);
13+
14+
// Track calls to the exposed function
15+
const clickEvent = await page.spyOnEvent('click');
16+
const input = page.locator('label.textarea-wrapper');
17+
18+
// Use position to make sure we click into the label enough to trigger
19+
// what would be the double click
20+
await input.click({
21+
position: {
22+
x: 5,
23+
y: 5,
24+
},
25+
});
26+
27+
// Verify the click was triggered exactly once
28+
expect(clickEvent).toHaveReceivedEventTimes(1);
29+
30+
// Verify that the event target is the checkbox and not the item
31+
const event = clickEvent.events[0];
32+
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-textarea');
33+
});
34+
});
35+
});

‎core/src/components/textarea/textarea.tsx

+13-1
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,18 @@ export class Textarea implements ComponentInterface {
572572
return this.label !== undefined || this.labelSlot !== null;
573573
}
574574

575+
/**
576+
* Stops propagation when the label is clicked,
577+
* otherwise, two clicks will be triggered.
578+
*/
579+
private onLabelClick = (ev: MouseEvent) => {
580+
// Only stop propagation if the click was directly on the label
581+
// and not on the input or other child elements
582+
if (ev.target === ev.currentTarget) {
583+
ev.stopPropagation();
584+
}
585+
};
586+
575587
/**
576588
* Renders the border container when fill="outline".
577589
*/
@@ -726,7 +738,7 @@ export class Textarea implements ComponentInterface {
726738
* interactable, clicking the label would focus that instead
727739
* since it comes before the textarea in the DOM.
728740
*/}
729-
<label class="textarea-wrapper" htmlFor={inputId}>
741+
<label class="textarea-wrapper" htmlFor={inputId} onClick={this.onLabelClick}>
730742
{this.renderLabelContainer()}
731743
<div class="textarea-wrapper-inner">
732744
{/**

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

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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 }, testInfo) => {
7+
testInfo.annotations.push({
8+
type: 'issue',
9+
description: 'https://github.com/ionic-team/ionic-framework/issues/30165',
10+
});
11+
12+
// Create a spy function in page context
13+
await page.setContent(`<ion-toggle onclick="console.log('click called')">my label</ion-toggle>`, config);
14+
15+
// Track calls to the exposed function
16+
let clickCount = 0;
17+
page.on('console', (msg) => {
18+
if (msg.text().includes('click called')) {
19+
clickCount++;
20+
}
21+
});
22+
23+
const input = page.locator('div.label-text-wrapper');
24+
25+
// Use position to make sure we click into the label enough to trigger
26+
// what would be the double click
27+
await input.click({
28+
position: {
29+
x: 5,
30+
y: 5,
31+
},
32+
});
33+
34+
// Verify the click was triggered exactly once
35+
expect(clickCount).toBe(1);
36+
});
37+
});
38+
});

‎core/src/components/toggle/toggle.tsx

+9
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)