refactor(item): do not automatically delegate focus

[liamdebeasi]

Issue number: resolves #21982

---

What is the current behavior?

History

delegatesFocus was originally added to ion-item so clicking an ion-label moves focus to the associated ion-input. When using the legacy form syntax, labels were not automatically associated with inputs. Given the following markup, clicking ion-label would not focus the input:

<ion-item>
  <ion-label>Input</ion-label>
  <ion-input></ion-input>
</ion-item>

delegatesFocus fixed this issue by moving focus to ion-input when a non-focusable part of the item is clicked. In this case, ion-label is non-focusable.

Problems with Delegates Focus

fortunately, delegatesFocus was also buggy in browsers such as WebKit. We needed to add workarounds which added technical debt to the project. We've slowly been able to remove these workarounds as the implementations in browsers improved.

In particular, we needed to add a workaround so that clicking the start/end padding in ion-item caused the input to be focused. The following screenshot demonstrates the start/end padding in the item. The color key is as follows:

White: ion-input

Blue: ion-label

Green: ion-item

Without delegatesFocus, clicking the green area does not cause the input to be focused. A user does not know about this implementation detail, so to them it seems like the input will sometimes not focus. In reality, this is happening because they sometimes tap the padding and other times they tap the label/input.

However, browser issues remain with this implementation. Developers have complained of unexpected focus and selection issues which resulted in even more technical debt added to try and resolve the issues.

Current Day

With the introduction of the new form control syntax, labels are automatically associated with their inputs. This means that when a label is clicked, the browser will automatically focus the associated input. This behavior is built in to the browser with label and input/textarea elements.

With the Entire item should be clickable with form controls feature we updated ion-item so that clicking the start/end padding focuses the input inside of the item.

Input/textarea were not accounted for in this scope, but by extending this functionality to input/textarea we would no longer need delegatesFocus for input/textarea.

What is the new behavior?

• Item no longer automatically delegates focus to the first focusable element
• Extended the Entire item should be clickable with form controls feature so clicking the padding focuses input/textarea.

Does this introduce a breaking change?

• Yes
• No

Other information

Design doc: https://github.com/ionic-team/ionic-framework-design-documents/commit/e09a8f309569a2a5383738d2bd727000b691871c

Only removing the delegatesFocus logic from ion-item causes focus trapping in ion-menu to fail. Upon further investigation I discovered that menu focus trapping only worked by chance. In our E2E test, we had an ion-item in a menu with an ion-button inside of it. There are a couple problems:

1. Menu moves focus to the ion-item because it has an .ion-focusable class. For some reason we consider the item focusable if it has an focusable elements inside of it. With the delegatesFocus logic, focus was being moved to the ion-button so the tests passed. However, this broke because focus was no longer delegated. The ion-item itself in this case is not a button item, so it cannot be focused. As a result, focus escaped from the menu. I'm not 100% sure why we consider an item to be focusable with this criteria, but I felt that changing that was too risky to be considered in scope.

2. I then tried to remove the ion-item from the test and just have an ion-button. I discovered something that I am confident is a bug which is that we always focus the host element and never try to focus the native button/input inside of a component's Shadow DOM. For the overlay focus trapping we have special logic to ensure the correct interactive element is focused. However, that logic does not exist in the menu. As a result, ion-button was being focused, but it's actually the native button inside of the shadow root that needs to be focused.

---

To fix this, I created a focus-trap file with utilities for focusing the first/last descendant in a focus trapped container. The implementation of this is the same implementation used in the overlay focus trapping. However, I moved it to this util file so that the menu can take advantage of that logic too (and therefore address the bug in point 2 above). There should be no behavior changes for the overlay components.

I removed the ion-item usage from the basic menu test which is why there are visual diffs.

[liamdebeasi]

The menu changes are required in order to ship the delegatesFocus change. However, I could move it to a separate PR if folks would prefer. I decided to keep them combined because we have solid test coverage with focus trapping, so I feel confident that this does not regress anything. Also, this is shipping in Ionic 8 so we'll have a beta period too.

[thetaPC]

LGTM, didn't have issues locally

[averyjohnston]

Nitpick: The padding around the input now has cursor: pointer, even though the rest of the control (label, input) doesn't; the inconsistency looks strange. For the other controls, it's consistent since cursor: pointer is used over the entire control.

[liamdebeasi]

Good catch. Any thoughts on how we could mitigate?

[averyjohnston]

That cursor: pointer is coming from the item-has-interactive-control class on ion-item, and then it gets overwritten by label's default styles. Either we could add cursor: pointer to the label --

/* possibly restricted to just labels in inputs/textareas */
.item-has-interactive-control label {
  cursor: pointer;
}

-- or make items with inputs/textareas not get cursor: pointer at all, likely by having it not add the class in that case. I'm not really sure which is best though 🤔

[liamdebeasi]

In 143fb7d I updated the code so that inputs and textures do not have the cursor effect. I updated the name of the class to be more descriptive of what it's doing, but I can change the name if you have other ideas.

[liamdebeasi]

I could also do this with CSS and have it be .item-has-interactive-control:not(:has(ion-input, ion-textarea)) but the specificity gets a little complicated, so I opted to just apply the class using logic in JS (since the class is already applied using JS logic)

[averyjohnston]

Maybe something like item-has-click-toggle-control or item-control-needs-pointer-cursor? "Needs cursor" by itself is confusing to me since in theory everything needs a cursor, like initially clicking into the input.

[liamdebeasi]

Clarified in 4362c3b and 0c9311c

[averyjohnston]

LGTM once the class name is adjusted. Might also want to update the firstInteractiveNeedsCursor variable to match.