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

Ace css issue using Input Method Editor (IME) #1845

Closed
shibayan opened this issue Dec 28, 2015 · 8 comments
Closed

Ace css issue using Input Method Editor (IME) #1845

shibayan opened this issue Dec 28, 2015 · 8 comments

Comments

@shibayan
Copy link
Contributor

I use Kudu ace editor under Input method editor env (standard use in japanese). I discovered that the display becomes strange as the image below.

aceissue

It seems following definitions it is in CSS is causing, but I can not determine whether there is no problem to remove. I need advice 😸

https://github.com/projectkudu/kudu/blob/master/Kudu.Services.Web/Content/Styles/FileBrowser.css#L165

@snobu
Copy link
Contributor

snobu commented Dec 28, 2015

min-height should never.. explode like that - and it's for the whole editor not for a single line, i think it's something else. How can i reproduce what you're seeing?

I'll take a look at it tomorrow morning.

@davidebbo
Copy link
Member

I'm guessing you need Japanese OS? I tried pasting some Japanese text from here into the editor, and nothing bad happened, so there is probably more to it.

Maybe it comes down to an ACE bug? @shibayan would be good to try if you can repro this with a trivial page that uses ACE and sets min-height.

@snobu
Copy link
Contributor

snobu commented Dec 28, 2015

Alright, so the min-height was introduced in this commit:
1f24ef9#diff-599dcd2b293b741d8bcf68cb863f548dR80

Not sure if this was meant to fix the older textarea-based editor or it serves a different purpose but i don't think Ace needs it at all since Ace sits directly inside a div, not a textarea.

Try to repro here: https://ace.c9.io/build/kitchen-sink.html

@snobu
Copy link
Contributor

snobu commented Dec 28, 2015

Ah, so Ace DOES use a textarea element for current line.

image

By the look of things this might be it. Let me do a bit of testing tomorrow to make sure the min-height doesn't affect anything else.

@shibayan
Copy link
Contributor Author

@davidebbo Yes, This issue require Japanese OS (and IME). In the case of copy and paste, it can not be reproduced.

I tried Ace kitchen-sink. This issue did not reproduce.
kitchen-sink

@snobu If there is no need in switching to Ace Editor, I want to delete this CSS definition.
Such as Japanese and only when IME is a required language, it seems this issue occurs.

ignore-minheight

If disable the min-height, it was confirmed to be displayed correctly. I hope if there is no effect on another element..

@snobu
Copy link
Contributor

snobu commented Dec 29, 2015

Ok, i'm fairly confident removing min-height from textarea won't negatively affect anything else.

Looks like there's a textarea in jquery-console as well, but i tested that, it doesn't care about min-height.

C:\seriouswork\kudu [AceIMEfix]> grep -r "<textarea" *
Kudu.Services.Web/Content/Scripts/jquery-console/jquery.console.js:             var typer = $('<textarea class="jquery-console-typer"></textarea>');
Kudu.Services.Web/Content/Scripts/jquery.contextMenu.js:                            $input = $('<textarea name=""></textarea>')

Looking though the rest of the source code, FileBrowser.css is only referenced in /DebugConsole/Default.cshtml
minheight

I think we're good. PR sent, #1846.

Thanks @shibayan for reporting this!

@shibayan
Copy link
Contributor Author

@snobu Thank you for code fix! 🎉 Kudu will be easier to use! (in IME required language)

@davidebbo
Copy link
Member

Thanks, it'll be in the next Kudu deployment.

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

No branches or pull requests

3 participants