Skip to content

Conversation

@n2
Copy link
Contributor

@n2 n2 commented Apr 7, 2021

Handle composition events to show IME suggestion on web, as described in #248. Sorry for being late, I have been busy in the past few days.

Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Thank you! And there is no late - we have no deadlines :)

undoer: Undoer<(CCursorPair, String)>,

// If IME candidate window is shown on this text edit.
has_candidate: bool,
Copy link
Owner

Choose a reason for hiding this comment

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

I think has_ime would be a more descriptive name

if !text_mark.is_empty()
&& text_mark != "\n"
&& text_mark != "\r"
&& state.has_candidate
Copy link
Owner

Choose a reason for hiding this comment

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

So we even need has_candidate? Is it for the case when the user switches between two TextEdit:s while an IME window is open? If so, it should be reset to false when the TextEdit loses focus.

Copy link
Contributor Author

@n2 n2 Apr 9, 2021

Choose a reason for hiding this comment

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

Yes, has_candidate is introduced exactly for that purpose. On my test platforms(Windows 10, Ubuntu 18.04, MacOs Big Sur and iOS 14.4), when user click other place of canvas, compositionend message is send, and such reset logit is done in compositionend handler.

Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Thanks 🙏

@emilk emilk merged commit 20bf095 into emilk:master Apr 12, 2021
@lunabunn lunabunn mentioned this pull request Apr 19, 2021
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.

2 participants