Skip to content
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

Clear text area after finalizing composition #4265

Closed
wants to merge 1 commit into from

Conversation

ink404
Copy link
Contributor

@ink404 ink404 commented Nov 15, 2022

Clear text area after finalizing composition

Previously stale text would display from text area in the composition helper when it heard the composition character. On some IMEs like GBoard this would manifest as the text propitiating after tapping delete, on others like the Samsung keyboard it would happen from tapping space. See attached videos for examples of before and after.

This PR clears the text area by setting _textArea.value = '' after finalizing the composition so when the composition is next started it does not contain the stale text and display it upon starting a new composition.

Previously I thought this change broke character composition, but after looking closer at Compostionhelper.test.ts and some additional manual testing to ensure everything works it seems the tests were checking for the presence of the uncleared _textArea text.

Before

Gboard Example:

before-en-pixel.mp4

Samsung Keyboard Example:

before-en-sm-kb.mp4

After

Samsung Keyboard (kr):

after-kr.mp4

Samsung Keyboard (en):

after-en.mp4

@ink404 ink404 marked this pull request as ready for review November 15, 2022 17:04
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

Isn't this case meant to be handled by currentCompositionPosition.start? I'm trying to get us to clear the textarea less and make it more complete via shell integration to improve screen reader accessibility of the prompt.

@Tyriar Tyriar marked this pull request as draft November 30, 2022 16:40
@Tyriar
Copy link
Member

Tyriar commented Dec 11, 2022

Closing for now as I believe this approach has negative impacts on screen reader accessibility.

@Tyriar Tyriar closed this Dec 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants