Skip to content

fix(scroll-assist): allow focus on input's siblings #30409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: feature-8.6
Choose a base branch
from

Conversation

thetaPC
Copy link
Contributor

@thetaPC thetaPC commented May 13, 2025

Issue number: internal


What is the current behavior?

If scroll assist is on (by default it is on for iOS mobile devices), then focus will be redirected to the input if a focusIn event fires within the component. This leads to some elements to not being accessible with screen readers.

For example: When the clear button is focused, focusin is dispatched and bubbles up to the ion-input. Our scroll assist utility listens for focusin to adjust the scroll position. It also causes the input to be re-focused. As a result, when swiping to the clear button with a screen reader, focus will be forcibly moved back to the input. This means that users cannot swipe away from the input to the right when using a screen reader.

This issue was resolved via PR #29366. However, this only fixed the issue with the clear button. Since then, ion-input has implemented start and end slots. This has opened up the possibility of other focusable elements to being present within ion-input. The same issue applies to them. A great example of this is the input-password-toggle. The password toggle is required to have a slot="end" to render appropriately within ion-input. But the users can never access the toggle when using a screen reader.

What is the new behavior?

Since we have no control of what is being passed within the the slots, the fix was done through the scroll assist instead of ion-input. It checks if any siblings of the native input have been focused, if they have then leave the focus on them instead of re-directing back to the native input.

  • Updated the scroll assist logic to check for focusable siblings of the native input. If found, then allow focus on them.
  • Removed the focusIn code from the clear button since it's being handle at the scroll assist level.

This fix will be introduced in a feature to lessen any possible issues in case of a regression.

Does this introduce a breaking change?

  • Yes
  • No

Other information

N/A

Copy link

vercel bot commented May 13, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ionic-framework ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 15, 2025 8:01pm

@github-actions github-actions bot added the package: core @ionic/core package label May 13, 2025
@thetaPC thetaPC marked this pull request as ready for review May 14, 2025 00:13
@thetaPC thetaPC requested a review from a team as a code owner May 14, 2025 00:13
@thetaPC thetaPC requested review from gnbm, ShaneK and brandyscarney and removed request for gnbm May 14, 2025 00:13
Copy link
Member

@ShaneK ShaneK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine to me and seems to work (assuming I'm testing correctly haha) but is there a reason it's going into the feature branch instead of main?

@thetaPC
Copy link
Contributor Author

thetaPC commented May 15, 2025

@ShaneK I'm not too familiar how the scroll assist works as a whole within Ionic Framework. How many components does this effect? Was there a reason that the fix for the clear button was done through the input instead at the scroll assist? etc. I figured that it would be safer to introduce it in a feature in case the fix ends up needing a regression. Thoughts?

@ShaneK
Copy link
Member

ShaneK commented May 15, 2025

@ShaneK I'm not too familiar how the scroll assist works as a whole within Ionic Framework. How many components does this effect? Was there a reason that the fix for the clear button was done through the input instead at the scroll assist? etc. I figured that it would be safer to introduce it in a feature in case the fix ends up needing a regression. Thoughts?

Sorry, I really don't have the context to answer this right now. @brandyscarney might when she's back? It's fair that it's better to be safe than sorry though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants