fix(input): clear button can be navigated using screen reader #29366
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
sean-perkins
approved these changes
Apr 23, 2024
Closed
3 tasks
liamdebeasi
added a commit
that referenced
this pull request
Apr 23, 2024
Issue number: resolves #29358 --------- <!-- 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. --> When the clear button is focused, `focusin` is dispatched and bubbles up to the `ion-input`. Our [scroll assist utility listens for `focusin`](https://github.com/ionic-team/ionic-framework/blob/2fc81ddc9b35d6909fd4b585079aedabbd659233/core/src/utils/input-shims/hacks/scroll-assist.ts#L135) 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. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - To fix this, I decided to have the `focusin` event from the clear button not bubble (as opposed to add a really specific workaround to the scroll assist utility). Adding `stopPropagation` was easy enough, but it turned out that the scroll assist listeners were all configured to listen during the capture phase instead of the bubble phase. As a result, `stopPropgation` had no effect because the scroll assist callback had already fired. To address this, I updated the listeners to listen during the bubbling phase instead of the capture phase. Based on my testing the capture phase was not required for scroll assist to work, so it appears safe to remove. ## 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.0.1-dev.11713535425.1a4afba3` Reviewers: Please test this on a physical iOS device and be sure to test the scroll assist behavior. There is a test at http://localhost:3333/src/utils/input-shims/hacks/test you can use.
liamdebeasi
added a commit
that referenced
this pull request
Apr 23, 2024
Dungchua
approved these changes
May 6, 2024
This was referenced May 22, 2024
This was referenced May 30, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue number: resolves #29358
What is the current behavior?
When the clear button is focused,
focusinis dispatched and bubbles up to theion-input. Our scroll assist utility listens forfocusinto 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.What is the new behavior?
focusinevent from the clear button not bubble (as opposed to add a really specific workaround to the scroll assist utility).Adding
stopPropagationwas easy enough, but it turned out that the scroll assist listeners were all configured to listen during the capture phase instead of the bubble phase. As a result,stopPropgationhad no effect because the scroll assist callback had already fired. To address this, I updated the listeners to listen during the bubbling phase instead of the capture phase. Based on my testing the capture phase was not required for scroll assist to work, so it appears safe to remove.Does this introduce a breaking change?
Other information
Dev build:
8.0.1-dev.11713535425.1a4afba3Reviewers: Please test this on a physical iOS device and be sure to test the scroll assist behavior. There is a test at http://localhost:3333/src/utils/input-shims/hacks/test you can use.