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

fix Korean(and Chinese, Japanese) IME behavior (#4541) #5615

Merged
merged 1 commit into from
May 4, 2016

Conversation

ioklo
Copy link
Contributor

@ioklo ioklo commented Apr 21, 2016

Hello, this PR is for issue #4541 - Korean characters problem

  • fix displaying Korean letters twice during composition
  • show composition letters between the text not over the text

Doing so, I added ICompositionEvent that has 'data' field. On 'compositionupdate' events, textarea is changed by composition data.

To show composition letters between the text, the visibleRange is calculated every compositionupdate event with composition word. It can be expensive.

I changed the variable shouldEmptyTextArea to be always true because I think textarea should be empty when composition starts but I didn't test with other browsers (How?).

Symptom (VSCode v1.0, type 'dkssudgktpdy')
animation1

Expected (there are two cursors.. it must be fix either)
animation2

And When I composite letters between the text, the letters are displayed over the text. But Korean IME doesn't act like that. (type 'gksrmf')
animation3

Expected
animation4

It is confused that Japanese IME usually show composition letters over the text (in the notepad) but when I tested with Microsoft Word, the composition letters are occupied between the text. (type 'sensei')
animation5

After change
animation6

But It's same as Microsoft Word
animation7

  - fix displaying Korean letters twice during composition
  - show composition letters between the text not over the text
@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @alexandrudima, @egamma and @joaomoreno to be potential reviewers

@msftclas
Copy link

Hi @ioklo, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

@ioklo ioklo changed the title #4541 fix Korean(and Chinese, Japanese) IME behavior fix Korean(and Chinese, Japanese) IME behavior (#4541) Apr 21, 2016
@msftclas
Copy link

@ioklo, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

@alexdima alexdima added this to the May 2016 milestone Apr 26, 2016
@alexdima alexdima merged commit 5939b5c into microsoft:master May 4, 2016
alexdima added a commit that referenced this pull request May 4, 2016
fix Korean(and Chinese, Japanese) IME behavior (#4541)
@alexdima
Copy link
Member

alexdima commented May 4, 2016

@ioklo Thank you very much for this change! ❤️

Based on a few issues and on this description I was able to put together an IME Smoke test that we will go through from now on any time we make changes in the input handling and your changes make VSCode work like Word in these test cases!

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.

4 participants