Skip to content

Commit 05e721d

Browse files
authored
refactor(item): do not automatically delegate focus (#29091)
resolves #21982 BREAKING CHANGE: - Item no longer automatically delegates focus to the first focusable element. While most developers should not need to make any changes to account for this update, usages of `ion-item` with interactive elements such as form controls (inputs, textareas, etc) should be evaluated to verify that interactions still work as expected.
1 parent 94c3ffc commit 05e721d

28 files changed

+181
-87
lines changed

BREAKING.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ This is a comprehensive list of the breaking changes introduced in the major ver
2222
- [Content](#version-8x-content)
2323
- [Datetime](#version-8x-datetime)
2424
- [Input](#version-8x-input)
25+
- [Item](#version-8x-item)
2526
- [Modal](#version-8x-modal)
2627
- [Nav](#version-8x-nav)
2728
- [Picker](#version-8x-picker)
@@ -168,6 +169,10 @@ For more information on the dynamic font, refer to the [Dynamic Font Scaling doc
168169
- `accept` has been removed from the `ion-input` component. This was previously used in conjunction with the `type="file"`. However, the `file` value for `type` is not a valid value in Ionic Framework.
169170
- The `legacy` property and support for the legacy syntax, which involved placing an `ion-input` inside of an `ion-item` with an `ion-label`, have been removed. For more information on migrating from the legacy input syntax, refer to the [Input documentation](https://ionicframework.com/docs/api/input#migrating-from-legacy-input-syntax).
170171

172+
<h4 id="version-8x-item">Item</h4>
173+
174+
- Item no longer automatically delegates focus to the first focusable element. While most developers should not need to make any changes to account for this update, usages of `ion-item` with interactive elements such as form controls (inputs, textareas, etc) should be evaluated to verify that interactions still work as expected.
175+
171176
<h4 id="version-8x-modal">Modal</h4>
172177

173178
- Detection for Capacitor <= 2 with applying status bar styles has been removed. Developers should ensure they are using Capacitor 3 or later when using the card modal presentation.

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,30 @@ configs().forEach(({ title, screenshot, config }) => {
4545
});
4646
});
4747
});
48+
49+
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
50+
test.describe(title('input: item functionality'), () => {
51+
test('clicking padded space within item should focus the input', async ({ page }) => {
52+
await page.setContent(
53+
`
54+
<ion-item>
55+
<ion-input label="Input"></ion-input>
56+
</ion-item>
57+
`,
58+
config
59+
);
60+
const itemNative = page.locator('.item-native');
61+
const input = page.locator('ion-input input');
62+
63+
// Clicks the padded space within the item
64+
await itemNative.click({
65+
position: {
66+
x: 5,
67+
y: 5,
68+
},
69+
});
70+
71+
await expect(input).toBeFocused();
72+
});
73+
});
74+
});

core/src/components/item/item.scss

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,8 @@
176176
// Item: Interactive
177177
// --------------------------------------------------
178178

179-
:host(.item-has-interactive-control) {
179+
// Inputs and textareas do not need the cursor, but other components like checkbox or toggle do.
180+
:host(.item-control-needs-pointer-cursor) {
180181
cursor: pointer;
181182
}
182183

core/src/components/item/item.tsx

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,7 @@ import type { CounterFormatter } from './item-interface';
3232
ios: 'item.ios.scss',
3333
md: 'item.md.scss',
3434
},
35-
shadow: {
36-
delegatesFocus: true,
37-
},
35+
shadow: true,
3836
})
3937
export class Item implements ComponentInterface, AnchorInterface, ButtonInterface {
4038
private labelColorStyles = {};
@@ -359,7 +357,7 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
359357

360358
private getFirstInteractive() {
361359
const controls = this.el.querySelectorAll<HTMLElement>(
362-
'ion-toggle:not([disabled]), ion-checkbox:not([disabled]), ion-radio:not([disabled]), ion-select:not([disabled])'
360+
'ion-toggle:not([disabled]), ion-checkbox:not([disabled]), ion-radio:not([disabled]), ion-select:not([disabled]), ion-input:not([disabled]), ion-textarea:not([disabled])'
363361
);
364362
return controls[0];
365363
}
@@ -425,7 +423,16 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
425423
*/
426424
const clickedWithinShadowRoot = this.el.shadowRoot!.contains(target);
427425
if (clickedWithinShadowRoot) {
428-
firstInteractive.click();
426+
/**
427+
* For input/textarea clicking the padding should focus the
428+
* text field (thus making it editable). For everything else,
429+
* we want to click the control so it activates.
430+
*/
431+
if (firstInteractive.tagName === 'ION-INPUT' || firstInteractive.tagName === 'ION-TEXTAREA') {
432+
(firstInteractive as HTMLIonInputElement | HTMLIonTextareaElement).setFocus();
433+
} else {
434+
firstInteractive.click();
435+
}
429436
}
430437
}
431438
}
@@ -441,6 +448,13 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
441448
const fillValue = fill || 'none';
442449
const inList = hostContext('ion-list', this.el) && !hostContext('ion-radio-group', this.el);
443450

451+
/**
452+
* Inputs and textareas do not need to show a cursor pointer.
453+
* However, other form controls such as checkboxes and radios do.
454+
*/
455+
const firstInteractiveNeedsPointerCursor =
456+
firstInteractive !== undefined && !['ION-INPUT', 'ION-TEXTAREA'].includes(firstInteractive.tagName);
457+
444458
return (
445459
<Host
446460
aria-disabled={ariaDisabled}
@@ -454,7 +468,7 @@ export class Item implements ComponentInterface, AnchorInterface, ButtonInterfac
454468
[`item-lines-${lines}`]: lines !== undefined,
455469
[`item-fill-${fillValue}`]: true,
456470
[`item-shape-${shape}`]: shape !== undefined,
457-
'item-has-interactive-control': firstInteractive !== undefined,
471+
'item-control-needs-pointer-cursor': firstInteractiveNeedsPointerCursor,
458472
'item-disabled': disabled,
459473
'in-list': inList,
460474
'item-multiple-inputs': this.multipleInputs,

core/src/components/menu/menu.tsx

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { ComponentInterface, EventEmitter } from '@stencil/core';
22
import { Build, Component, Element, Event, Host, Listen, Method, Prop, State, Watch, h } from '@stencil/core';
33
import { getTimeGivenProgression } from '@utils/animation/cubic-bezier';
4+
import { focusFirstDescendant, focusLastDescendant } from '@utils/focus-trap';
45
import { GESTURE_CONTROLLER } from '@utils/gesture';
56
import { shoudUseCloseWatcher } from '@utils/hardware-back-button';
67
import type { Attributes } from '@utils/helpers';
@@ -19,8 +20,6 @@ const iosEasing = 'cubic-bezier(0.32,0.72,0,1)';
1920
const mdEasing = 'cubic-bezier(0.0,0.0,0.2,1)';
2021
const iosEasingReverse = 'cubic-bezier(1, 0, 0.68, 0.28)';
2122
const mdEasingReverse = 'cubic-bezier(0.4, 0, 0.6, 1)';
22-
const focusableQueryString =
23-
'[tabindex]:not([tabindex^="-"]), input:not([type=hidden]):not([tabindex^="-"]), textarea:not([tabindex^="-"]), button:not([tabindex^="-"]), select:not([tabindex^="-"]), .ion-focusable:not([tabindex^="-"])';
2423

2524
/**
2625
* @part container - The container for the menu content.
@@ -398,31 +397,9 @@ export class Menu implements ComponentInterface, MenuI {
398397
return menuController._setOpen(this, shouldOpen, animated);
399398
}
400399

401-
private focusFirstDescendant() {
402-
const { el } = this;
403-
const firstInput = el.querySelector(focusableQueryString) as HTMLElement | null;
404-
405-
if (firstInput) {
406-
firstInput.focus();
407-
} else {
408-
el.focus();
409-
}
410-
}
411-
412-
private focusLastDescendant() {
413-
const { el } = this;
414-
const inputs = Array.from(el.querySelectorAll<HTMLElement>(focusableQueryString));
415-
const lastInput = inputs.length > 0 ? inputs[inputs.length - 1] : null;
416-
417-
if (lastInput) {
418-
lastInput.focus();
419-
} else {
420-
el.focus();
421-
}
422-
}
423-
424400
private trapKeyboardFocus(ev: Event, doc: Document) {
425401
const target = ev.target as HTMLElement | null;
402+
426403
if (!target) {
427404
return;
428405
}
@@ -439,13 +416,15 @@ export class Menu implements ComponentInterface, MenuI {
439416
* Wrap the focus to either the first or last element.
440417
*/
441418

419+
const { el } = this;
420+
442421
/**
443422
* Once we call `focusFirstDescendant`, another focus event
444423
* will fire, which will cause `lastFocus` to be updated
445424
* before we can run the code after that. We cache the value
446425
* here to avoid that.
447426
*/
448-
this.focusFirstDescendant();
427+
focusFirstDescendant(el);
449428

450429
/**
451430
* If the cached last focused element is the same as the now-
@@ -454,7 +433,7 @@ export class Menu implements ComponentInterface, MenuI {
454433
* last descendant.
455434
*/
456435
if (this.lastFocus === doc.activeElement) {
457-
this.focusLastDescendant();
436+
focusLastDescendant(el);
458437
}
459438
}
460439
}

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,7 @@
3535
</ion-header>
3636
<ion-content>
3737
<ion-list>
38-
<ion-item>
39-
<ion-button id="start-menu-button">Button</ion-button>
40-
</ion-item>
38+
<ion-button id="start-menu-button">Button</ion-button>
4139
<ion-item>Menu Item</ion-item>
4240
<ion-item>Menu Item</ion-item>
4341
<ion-item>Menu Item</ion-item>
123 Bytes
Loading
212 Bytes
Loading
383 Bytes
Loading
111 Bytes
Loading

0 commit comments

Comments
 (0)