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

fine tune when native text area is updated #198153

Merged
merged 5 commits into from
Nov 21, 2023
Merged

Conversation

meganrogge
Copy link
Contributor

@meganrogge meganrogge commented Nov 13, 2023

The function called writeScreenReaderContent was also used for other things:

// Clear the textarea to avoid an unwanted cursor type
this.writeNativeTextAreaContent('blurWithoutCompositionEnd');

I have renamed it writeNativeTextAreaContent as that's what I believe it represents.

Skipping the update when the reason is render when not in screen reader mode has performance benefits and doesn't break anything. I've tried other variants and it breaks smoke tests.

fix #192278

@meganrogge meganrogge self-assigned this Nov 13, 2023
@meganrogge meganrogge added this to the November 2023 milestone Nov 13, 2023
@meganrogge
Copy link
Contributor Author

Looks like returning on render breaks only Korean 🤔 . Is there a way we can detect that's the current language?

Tyriar
Tyriar previously requested changes Nov 14, 2023
src/vs/editor/browser/controller/textAreaInput.ts Outdated Show resolved Hide resolved
src/vs/editor/browser/controller/textAreaInput.ts Outdated Show resolved Hide resolved
src/vs/editor/browser/controller/textAreaInput.ts Outdated Show resolved Hide resolved
src/vs/editor/browser/controller/textAreaInput.ts Outdated Show resolved Hide resolved
@meganrogge meganrogge changed the title for some events, only writeScreenReaderContent when screen reader is enabled only update native text area content on render when in screen reader mode Nov 14, 2023
@meganrogge meganrogge changed the title only update native text area content on render when in screen reader mode only update native text area content when in screen reader mode Nov 14, 2023
@meganrogge meganrogge marked this pull request as draft November 14, 2023 19:57
@meganrogge meganrogge changed the title only update native text area content when in screen reader mode fine tune when native text area is updated Nov 14, 2023
@meganrogge meganrogge marked this pull request as ready for review November 14, 2023 20:14
@hediet hediet removed their request for review November 20, 2023 10:54
@hediet
Copy link
Member

hediet commented Nov 20, 2023

Code change looks good, but I don't have enough context to do a proper review.

@meganrogge
Copy link
Contributor Author

@hediet Would @alexdima ?

Or are there questions I can answer?

This is also not my code area but seemed fine based on my testing.

@hediet
Copy link
Member

hediet commented Nov 21, 2023

Lets merge and test properly.

@meganrogge meganrogge merged commit 1ffd522 into main Nov 21, 2023
6 checks passed
@meganrogge meganrogge deleted the merogge/write-screen-reader branch November 21, 2023 15:57
@github-actions github-actions bot locked and limited conversation to collaborators Jan 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Selecting of large folded regions is slow. Time spent in writeScreenReaderContent
3 participants