Skip to content

Commit 54a1c86

Browse files
thetaPCIonitronShaneK
authored
fix(checkbox, toggle): fire ionFocus and ionBlur (#30733)
Issue number: internal --------- <!-- 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. --> `ionFocus` and `ionBlur` are not being emitted for checkbox and toggle. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Moved the `onFocus` and `onBlur` to `Host` - Added tests for `onFocus`, `onBlur`, and `onChange`. - Added a workaround on Playwright in order to trigger `ionBlur` since Playwright browsers aren't acting like native browsers when it comes to tabbing. ## 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. --> Dev build: `8.7.7-dev.11761071592.1d1b804d` --------- Co-authored-by: ionitron <hi@ionicframework.com> Co-authored-by: Shane <shane@shanessite.net>
1 parent ba73988 commit 54a1c86

21 files changed

+542
-19
lines changed

core/src/components/checkbox/checkbox.tsx

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ export class Checkbox implements ComponentInterface {
3434
private inputLabelId = `${this.inputId}-lbl`;
3535
private helperTextId = `${this.inputId}-helper-text`;
3636
private errorTextId = `${this.inputId}-error-text`;
37-
private focusEl?: HTMLElement;
3837
private inheritedAttributes: Attributes = {};
3938

4039
@Element() el!: HTMLIonCheckboxElement;
@@ -147,9 +146,7 @@ export class Checkbox implements ComponentInterface {
147146
/** @internal */
148147
@Method()
149148
async setFocus() {
150-
if (this.focusEl) {
151-
this.focusEl.focus();
152-
}
149+
this.el.focus();
153150
}
154151

155152
/**
@@ -169,7 +166,6 @@ export class Checkbox implements ComponentInterface {
169166
private toggleChecked = (ev: Event) => {
170167
ev.preventDefault();
171168

172-
this.setFocus();
173169
this.setChecked(!this.checked);
174170
this.indeterminate = false;
175171
};
@@ -285,6 +281,9 @@ export class Checkbox implements ComponentInterface {
285281
aria-disabled={disabled ? 'true' : null}
286282
tabindex={disabled ? undefined : 0}
287283
onKeyDown={this.onKeyDown}
284+
onFocus={this.onFocus}
285+
onBlur={this.onBlur}
286+
onClick={this.onClick}
288287
class={createColorClasses(color, {
289288
[mode]: true,
290289
'in-item': hostContext('ion-item', el),
@@ -296,7 +295,6 @@ export class Checkbox implements ComponentInterface {
296295
[`checkbox-alignment-${alignment}`]: alignment !== undefined,
297296
[`checkbox-label-placement-${labelPlacement}`]: true,
298297
})}
299-
onClick={this.onClick}
300298
>
301299
<label class="checkbox-wrapper" htmlFor={inputId}>
302300
{/*
@@ -309,9 +307,6 @@ export class Checkbox implements ComponentInterface {
309307
disabled={disabled}
310308
id={inputId}
311309
onChange={this.toggleChecked}
312-
onFocus={() => this.onFocus()}
313-
onBlur={() => this.onBlur()}
314-
ref={(focusEl) => (this.focusEl = focusEl)}
315310
required={required}
316311
{...inheritedAttributes}
317312
/>

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

Lines changed: 195 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@ configs().forEach(({ title, screenshot, config }) => {
4444
});
4545
});
4646

47-
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
47+
/**
48+
* This behavior does not vary across modes/directions
49+
*/
50+
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
4851
test.describe(title('checkbox: ionChange'), () => {
4952
test('should fire ionChange when interacting with checkbox', async ({ page }) => {
5053
await page.setContent(
@@ -133,4 +136,195 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
133136
expect(clickCount).toBe(1);
134137
});
135138
});
139+
140+
test.describe(title('checkbox: ionFocus'), () => {
141+
test('should not have visual regressions', async ({ page, pageUtils }) => {
142+
await page.setContent(
143+
`
144+
<style>
145+
#container {
146+
display: inline-block;
147+
padding: 10px;
148+
}
149+
</style>
150+
151+
<div id="container">
152+
<ion-checkbox>Unchecked</ion-checkbox>
153+
</div>
154+
`,
155+
config
156+
);
157+
158+
await pageUtils.pressKeys('Tab');
159+
160+
const container = page.locator('#container');
161+
162+
await expect(container).toHaveScreenshot(screenshot(`checkbox-focus`));
163+
});
164+
165+
test('should not have visual regressions when interacting with checkbox in item', async ({ page, pageUtils }) => {
166+
await page.setContent(
167+
`
168+
<ion-item class="ion-focused">
169+
<ion-checkbox>Unchecked</ion-checkbox>
170+
</ion-item>
171+
`,
172+
config
173+
);
174+
175+
// Test focus with keyboard navigation
176+
await pageUtils.pressKeys('Tab');
177+
178+
const item = page.locator('ion-item');
179+
180+
await expect(item).toHaveScreenshot(screenshot(`checkbox-in-item-focus`));
181+
});
182+
183+
test('should fire ionFocus when checkbox is focused', async ({ page, pageUtils }) => {
184+
await page.setContent(
185+
`
186+
<ion-checkbox aria-label="checkbox" value="my-checkbox"></ion-checkbox>
187+
`,
188+
config
189+
);
190+
191+
const ionFocus = await page.spyOnEvent('ionFocus');
192+
193+
// Test focus with keyboard navigation
194+
await pageUtils.pressKeys('Tab');
195+
196+
expect(ionFocus).toHaveReceivedEventTimes(1);
197+
198+
// Reset focus
199+
const checkbox = page.locator('ion-checkbox');
200+
const checkboxBoundingBox = (await checkbox.boundingBox())!;
201+
await page.mouse.click(0, checkboxBoundingBox.height + 1);
202+
203+
// Test focus with click
204+
await checkbox.click();
205+
206+
expect(ionFocus).toHaveReceivedEventTimes(2);
207+
});
208+
209+
test('should fire ionFocus when interacting with checkbox in item', async ({ page, pageUtils }) => {
210+
await page.setContent(
211+
`
212+
<ion-item>
213+
<ion-checkbox aria-label="checkbox" value="my-checkbox"></ion-checkbox>
214+
</ion-item>
215+
`,
216+
config
217+
);
218+
219+
const ionFocus = await page.spyOnEvent('ionFocus');
220+
221+
// Test focus with keyboard navigation
222+
await pageUtils.pressKeys('Tab');
223+
224+
expect(ionFocus).toHaveReceivedEventTimes(1);
225+
226+
// Verify that the event target is the checkbox and not the item
227+
const eventByKeyboard = ionFocus.events[0];
228+
expect((eventByKeyboard.target as HTMLElement).tagName.toLowerCase()).toBe('ion-checkbox');
229+
230+
// Reset focus
231+
const checkbox = page.locator('ion-checkbox');
232+
const checkboxBoundingBox = (await checkbox.boundingBox())!;
233+
await page.mouse.click(0, checkboxBoundingBox.height + 1);
234+
235+
// Test focus with click
236+
const item = page.locator('ion-item');
237+
await item.click();
238+
239+
expect(ionFocus).toHaveReceivedEventTimes(2);
240+
241+
// Verify that the event target is the checkbox and not the item
242+
const eventByClick = ionFocus.events[0];
243+
expect((eventByClick.target as HTMLElement).tagName.toLowerCase()).toBe('ion-checkbox');
244+
});
245+
246+
test('should not fire when programmatically setting a value', async ({ page }) => {
247+
await page.setContent(
248+
`
249+
<ion-checkbox aria-label="checkbox" value="my-checkbox"></ion-checkbox>
250+
`,
251+
config
252+
);
253+
254+
const ionFocus = await page.spyOnEvent('ionFocus');
255+
const checkbox = page.locator('ion-checkbox');
256+
257+
await checkbox.evaluate((el: HTMLIonCheckboxElement) => (el.checked = true));
258+
expect(ionFocus).not.toHaveReceivedEvent();
259+
});
260+
});
261+
262+
test.describe(title('checkbox: ionBlur'), () => {
263+
test('should fire ionBlur when checkbox is blurred', async ({ page, pageUtils }) => {
264+
await page.setContent(
265+
`
266+
<ion-checkbox aria-label="checkbox" value="my-checkbox"></ion-checkbox>
267+
`,
268+
config
269+
);
270+
271+
const ionBlur = await page.spyOnEvent('ionBlur');
272+
273+
// Test blur with keyboard navigation
274+
// Focus the checkbox
275+
await pageUtils.pressKeys('Tab');
276+
// Blur the checkbox
277+
await pageUtils.pressKeys('Tab');
278+
279+
expect(ionBlur).toHaveReceivedEventTimes(1);
280+
281+
// Test blur with click
282+
const checkbox = page.locator('ion-checkbox');
283+
// Focus the checkbox
284+
await checkbox.click();
285+
// Blur the checkbox by clicking outside of it
286+
const checkboxBoundingBox = (await checkbox.boundingBox())!;
287+
await page.mouse.click(0, checkboxBoundingBox.height + 1);
288+
289+
expect(ionBlur).toHaveReceivedEventTimes(2);
290+
});
291+
292+
test('should fire ionBlur after interacting with checkbox in item', async ({ page, pageUtils }) => {
293+
await page.setContent(
294+
`
295+
<ion-item>
296+
<ion-checkbox aria-label="checkbox" value="my-checkbox"></ion-checkbox>
297+
</ion-item>
298+
`,
299+
config
300+
);
301+
302+
const ionBlur = await page.spyOnEvent('ionBlur');
303+
304+
// Test blur with keyboard navigation
305+
// Focus the checkbox
306+
await pageUtils.pressKeys('Tab');
307+
// Blur the checkbox
308+
await pageUtils.pressKeys('Tab');
309+
310+
expect(ionBlur).toHaveReceivedEventTimes(1);
311+
312+
// Verify that the event target is the checkbox and not the item
313+
const event = ionBlur.events[0];
314+
expect((event.target as HTMLElement).tagName.toLowerCase()).toBe('ion-checkbox');
315+
316+
// Test blur with click
317+
const item = page.locator('ion-item');
318+
await item.click();
319+
// Blur the checkbox by clicking outside of it
320+
const itemBoundingBox = (await item.boundingBox())!;
321+
await page.mouse.click(0, itemBoundingBox.height + 1);
322+
323+
expect(ionBlur).toHaveReceivedEventTimes(2);
324+
325+
// Verify that the event target is the checkbox and not the item
326+
const eventByClick = ionBlur.events[0];
327+
expect((eventByClick.target as HTMLElement).tagName.toLowerCase()).toBe('ion-checkbox');
328+
});
329+
});
136330
});
1.97 KB
Loading
2.38 KB
Loading
2.78 KB
Loading
2.04 KB
Loading
2.71 KB
Loading
2.63 KB
Loading

core/src/components/checkbox/test/basic/index.html

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,20 @@
5050
<ion-checkbox checked style="width: 200px">Specified width</ion-checkbox><br />
5151
<ion-checkbox checked style="width: 100%">Full-width</ion-checkbox><br />
5252
</ion-content>
53+
54+
<script>
55+
document.addEventListener('ionBlur', (ev) => {
56+
console.log('ionBlur', ev);
57+
});
58+
59+
document.addEventListener('ionChange', (ev) => {
60+
console.log('ionChange', ev);
61+
});
62+
63+
document.addEventListener('ionFocus', (ev) => {
64+
console.log('ionFocus', ev);
65+
});
66+
</script>
5367
</ion-app>
5468
</body>
5569
</html>

core/src/components/checkbox/test/item/index.html

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,20 @@ <h1>Multiline Label</h1>
246246
</div>
247247
</div>
248248
</ion-content>
249+
250+
<script>
251+
document.addEventListener('ionBlur', (ev) => {
252+
console.log('ionBlur', ev);
253+
});
254+
255+
document.addEventListener('ionChange', (ev) => {
256+
console.log('ionChange', ev);
257+
});
258+
259+
document.addEventListener('ionFocus', (ev) => {
260+
console.log('ionFocus', ev);
261+
});
262+
</script>
249263
</ion-app>
250264
</body>
251265
</html>

0 commit comments

Comments
 (0)