Basic IME mapping #204
Basic IME mapping #204
Conversation
src/systems.rs
Outdated
| match ev { | ||
| Ime::Preedit { | ||
| window, | ||
| value: _, |
There was a problem hiding this comment.
shouldn't we also push a CompositionUpdate event with this value right after pushing CompositionStart?
(looking at the reference implementation at https://github.com/emilk/egui/blob/2bc2fb9c393bbbfc4832a35af1c100bc4ea48719/crates/egui-winit/src/lib.rs#L301)
There was a problem hiding this comment.
Sounds good to me! I read through the reference and all code that uses the event. The event is used by TextEdit to preview text before it's committed by the IME. It should also be added for the sake of completion.
I added it here: 40e4236
There was a problem hiding this comment.
Sorry for forgetting to comment on this again, I feel really bad for neglecting this PR for several releases. The last question I want to ask is if there is a reason why we don't align our implementation to the reference one completely.
I haven't had a chance to test it myself and to understand if there's really any difference in behaviour, but I guess I'd feel safer if we just adopted exactly the same logic.
|
Hey! Thank you for the PR and apologies for taking a while to review it. Looks good to me in general, left 1 comment about the |
Co-authored-by: mvlabat <[email protected]>
Fixes #145
This is enough for my needs, but FYI:
Ime::Disabledjust maps toEvent::CompositionEndwith an empty string for text. You could captureIme::Preeditas state to handle it that way. I don't know where best to add the state.