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 IME problem - especially for CJKT. #11

Closed
wants to merge 1 commit into from

Conversation

iLoveBug
Copy link

Works perfect on Chrome.
Known issue for Safari: the first chinese world is duplicated.

mobiledoc-kit-IME-fix

Verified OK on Chrome.
Known issue for Safari: the first chinese world is duplicated.
@huaying
Copy link

huaying commented Dec 19, 2019

Hi I just did some tests for your fix.
Do you know which repo is a right place to fire a fix by the way?

ezgif-1-1b8804f9c119

@iLoveBug
Copy link
Author

Hi I just did some tests for your fix.
Do you know which repo is a right place to fire a fix by the way?

ezgif-1-1b8804f9c119

Not sure.

It seems the CI check test failed. Maybe we need more test.

@iLoveBug
Copy link
Author

iLoveBug commented Dec 19, 2019

I found the ci test failed mainly due to the safari environment problem.
截屏2019-12-19下午5 02 08

Do you mean I should raise the pull request to the root repo bustle/mobiledoc-kit?

@@ -210,6 +217,43 @@ export default class EventManager {
this._updateModifiersFromKey(key, {isDown:false});
}

compositionstart() {
Copy link

Choose a reason for hiding this comment

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

why not use camelCase instead?

Suggested change
compositionstart() {
compositionStart() {

Copy link

Choose a reason for hiding this comment

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

oops, that's the event name.

@wuliaopage
Copy link

Works perfect on Chrome.
Known issue for Safari: the first chinese world is duplicated.

mobiledoc-kit-IME-fix

Thanks so much for PR. Can you please look into this PR and repair that checks error. Many people be appreciated for your contribution. This would be a big fix for ghost for years.

@iLoveBug
Copy link
Author

Thanks so much for PR. Can you please look into this PR and repair that checks error. Many people be appreciated for your contribution. This would be a big fix for ghost for years.

Frankly speaking, I have no idea about the errors on CI test. It seems it's an environment problem. Anybody can help?

截屏2020-01-29上午9 53 25

截屏2020-01-29上午9 53 56

@Dixeran
Copy link

Dixeran commented Mar 15, 2020

Thanks so much for PR. Can you please look into this PR and repair that checks error. Many people be appreciated for your contribution. This would be a big fix for ghost for years.

Frankly speaking, I have no idea about the errors on CI test. It seems it's an environment problem. Anybody can help?

截屏2020-01-29上午9 53 25 截屏2020-01-29上午9 53 56

By doing some search, it seems like probably because some temporary failed of saucelab. I don't know if it is appropriate to submit it (retry the CI test) again?

@iLoveBug
Copy link
Author

How to do the re-submit?

@bin16
Copy link

bin16 commented May 4, 2020

@ErisDS Hi, could you please redo the checks manually? https://travis-ci.com/github/TryGhost/mobiledoc-kit/builds/141700597#L1120 the logs says it failed to request IE11/Edge/Safari environment so that the CI test ended with 3 fails

Some lines of logs:

not ok 594 SL_MS_Edge - [undefined ms] - bailout
not ok 602 SL_Safari_Current - [undefined ms] - bailout
not ok 603 SL_IE_11 - [undefined ms] - bailout
The environment you requested was unavailable.
...
1..2215
# tests 2215
# pass  2204
# skip  8
# fail  3
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
The command "yarn test:ci" exited with 1.
store build cache

Done. Your build exited with 1.

Thanks a lot!

@kevinansfield
Copy link
Collaborator

Please keep non-Ghost-specific PRs on the upstream mobiledoc-kit repository, thank you 🙂

CI will not run on this fork. It's only used for Ghost-specific changes, otherwise it's kept in sync with the main mobiledoc-kit repo.

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.

7 participants