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

Set correct text size for preedit window #2790

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rhysd
Copy link
Contributor

@rhysd rhysd commented Feb 5, 2025

Problem

Currently the text size of preedit window is fixed to renderer.default_size(). However the text size of text input and text editor can be changed by user. Using same size for preedit window is more natural.

before.mp4

Solution

This PR propagates the text size from the widget which requests IME to the window manager which manages the event loop. The propagated text size is used when the preedit window is rendered if available.

after.mp4

@rhysd rhysd force-pushed the adjust-preedit-size branch from 516b5f4 to fcdf53a Compare February 5, 2025 17:02
@rhysd rhysd changed the title Set correct text size for text in preedit window Set correct text size for preedit window Feb 5, 2025
Comment on lines 43 to 50
pub fn new(text_size: Option<impl Into<Pixels>>) -> Self
where
T: Default,
{
Self::default()
Self {
text_size: text_size.map(Into::into),
..Default::default()
}
Copy link
Member

Choose a reason for hiding this comment

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

No need. Let's not ask for any argument and just use Default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I'll make a change.

Copy link
Contributor Author

@rhysd rhysd Feb 6, 2025

Choose a reason for hiding this comment

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

@hecrj I addressed this at cf851e1

@kenz-gelsoft
Copy link
Contributor

kenz-gelsoft commented Feb 5, 2025

Thank you for working on follow-ups!

I had an idea on this, which text widgets pass the caret height as a preedit style hint.

Passing text style itself achieves high fidelity preedit, but we need to find balance to widgets requirement.

@rhysd
Copy link
Contributor Author

rhysd commented Feb 6, 2025

@kenz-gelsoft Yeah, it's even better idea but it would require more design decisions. In this PR, I'd like to focus on only the text size as a quick fix. BTW let me say thank you for the bunch of the first work for adding IME support. It's definitely needed for CJK users 👍

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.

3 participants