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

Fixed Japanese IME popup position #940

Merged
merged 1 commit into from Jan 5, 2018

Conversation

Projects
None yet
2 participants
@tamurashingo
Contributor

tamurashingo commented Jan 4, 2018

closes TryGhost/Ghost#9289

  • add css to fix hidden textarea position

found bug

Japanese IME popup position.
34098096-9f24a208-e41e-11e7-859e-9ca2b51e082e

hidden textarea size and position.
34098036-70c695d8-e41e-11e7-8ac3-97683cf998b7

fix version

Japanese IME popup position.
2018-01-04 19 22 37

hidden textarea size and position.
2018-01-04 19 23 01

Please include a description of your change & check your PR against this list, thanks!

  • Commit message has a short title & issue references
  • Commits are squashed
  • The build will pass (run ember test from the repo root - will be core/client if working from the submodule in Ghost).
Fixed Japanese IME popup position
closes TryGhost/Ghost#9289
- add css to fix hidden textarea position
@kevinansfield

This comment has been minimized.

Collaborator

kevinansfield commented Jan 4, 2018

Hey @tamurashingo 👋 Thanks for taking a look at this!

Quick question, how does the IME input behave when the editor text size varies due to header text or mobile screen sizes?

screen shot 2018-01-04 at 11 07 37

screen shot 2018-01-04 at 11 10 28

It would be good to see if there's a more generic styling approach, CodeMirror is also used on the Code Injection screen and in the code injection panel of the post settings menu, both of which will be affected by our generic textarea style.

@tamurashingo

This comment has been minimized.

Contributor

tamurashingo commented Jan 5, 2018

Hi, @kevinansfield Thanks for comment!
I checked the IME behavior in header and mobile.

header

header

This is not perfect.
In header, it will be good that textarea bottom position is the same as the bottom of the header.
(or font size in textarea is the same as header)

CodeMirror and SimpleMDE have same problem.
I think this header problem should be owned by CodeMirror or SimpleMDE.

CodeMirror:
CodeMirror

SimpleMDE:
SimpleMDE

mobile

mobile screen size

It seems good.

In this PR size is written by absolute value, but it seems that written by relative value is better.
like:

.gh-editor .CodeMirror-wrap > div > textarea {
    top: 0;
    height: 1.6rem;
    min-height: 1.6rem;
    margin-bottom: -1.6rem;
}

this approach, media query can be used.

@media (max-width: 960px) {
    .gh-editor .CodeMirror-wrap > div > textarea {
        height: 1.4rem;
        ......
    }
}
@kevinansfield

This comment has been minimized.

Collaborator

kevinansfield commented Jan 5, 2018

CodeMirror and SimpleMDE have same problem.
I think this header problem should be owned by CodeMirror or SimpleMDE.

Agreed, the header issue should probably be raised on CodeMirror's repo.

Happy to merge as-is with the fixed height, thanks! 😄

@kevinansfield kevinansfield merged commit 4baa57b into TryGhost:master Jan 5, 2018

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment