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.

Sign up for GitHub

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

Jump to bottom

Windows: Add composition event support #2241

Merged
merged 29 commits into from
Apr 13, 2022
Merged

Windows: Add composition event support #2241

merged 29 commits into from
Apr 13, 2022

Conversation

moko256
Copy link
Contributor

@moko256 moko256 commented Apr 5, 2022 •
edited

This PR will add composition event support.
Related: #1497

  • IME events for Windows
  • Window::set_ime_allowed() for Windows
  • Breaking: Hide OS-drawn composing text for Windows (same as the other platforms)

Tested on ime example (feel free to comment if you try this PR):

  • Microsoft IME - Japanese (on Windows 10 19044.1620)
  • Google Japanese Input - Japanese (on Windows 10 19044.1620)
  • ATOK - Japanese (on Windows 10 19044.1620)

Tested on ime example but I am not native

  • Microsoft IME - Korean (on Windows 10 19044.1620)
  • Microsoft IME Pinyin - Chinese (Simplified, China) (on Windows 10 19044.1620)
  • (Microsoft) Vietnamse Telex - Vietnamse (on Windows 10 19044.1620)
  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

@kchibisov kchibisov force-pushed the composition-event branch 3 times, most recently from 8e966e6 to 5db61c1 Compare April 6, 2022 10:12
@moko256 moko256 force-pushed the moko256_windows_composition_event branch from c1d660c to 59edbd1 Compare April 7, 2022 19:05
@moko256 moko256 force-pushed the moko256_windows_composition_event branch from ef8b87d to 9532e59 Compare April 7, 2022 19:15
@moko256 moko256 mentioned this pull request Apr 7, 2022
Open
12 tasks
@moko256 moko256 marked this pull request as ready for review April 8, 2022 14:14
kchibisov
kchibisov requested changes Apr 9, 2022
View reviewed changes
Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

Thanks for working on Windows parts of the API. I have no clue about windows and I don't have any machine running it, however I'll give you a code style review, so when one of the windows maintainers will look into that the changes should be impl wise and not code style, I hope.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
src/platform_impl/windows/event_loop.rs Outdated Show resolved Hide resolved
src/platform_impl/windows/event_loop.rs Outdated Show resolved Hide resolved
src/platform_impl/windows/ime.rs Outdated Show resolved Hide resolved
src/platform_impl/windows/ime.rs Outdated Show resolved Hide resolved
src/platform_impl/windows/ime.rs Outdated Show resolved Hide resolved
src/platform_impl/windows/ime.rs Outdated Show resolved Hide resolved
src/platform_impl/windows/ime.rs Outdated Show resolved Hide resolved
src/platform_impl/windows/event_loop.rs Outdated Show resolved Hide resolved
moko256 and others added 3 commits April 10, 2022 10:52
@moko256 @kchibisov
fix typo
841ba77 
Co-authored-by: Kirill Chibisov <contact@kchibisov.com>
@moko256 @kchibisov
fix some codes from review
c301e00 
Co-authored-by: Kirill Chibisov <contact@kchibisov.com>
@moko256 @kchibisov
use is_none and is_some
4312af9 
Co-authored-by: Kirill Chibisov <contact@kchibisov.com>
kchibisov
kchibisov requested changes Apr 11, 2022
View reviewed changes
Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

It looks better, just some minor comments.

src/platform_impl/windows/ime.rs Outdated Show resolved Hide resolved
src/platform_impl/windows/ime.rs Show resolved Hide resolved
src/platform_impl/windows/ime.rs Outdated Show resolved Hide resolved
src/platform_impl/windows/ime.rs Outdated Show resolved Hide resolved
src/platform_impl/windows/ime.rs Outdated Show resolved Hide resolved
src/platform_impl/windows/ime.rs Outdated Show resolved Hide resolved
src/platform_impl/windows/ime.rs Outdated Show resolved Hide resolved
src/platform_impl/windows/ime.rs Outdated Show resolved Hide resolved
moko256 and others added 8 commits April 12, 2022 03:02
@moko256 @kchibisov
improve line break
85727ae 
Co-authored-by: Kirill Chibisov <contact@kchibisov.com>
@moko256 @kchibisov
improve line break
12bdb1b 
Co-authored-by: Kirill Chibisov <contact@kchibisov.com>
@moko256 @kchibisov
improve line break
ac3f74e 
Co-authored-by: Kirill Chibisov <contact@kchibisov.com>
@moko256 @kchibisov
improve line break
1b40f3e 
Co-authored-by: Kirill Chibisov <contact@kchibisov.com>
@moko256 @kchibisov
improve line break
bc7b60b 
Co-authored-by: Kirill Chibisov <contact@kchibisov.com>
@moko256 @kchibisov
improve if usage
b7934bc 
Co-authored-by: Kirill Chibisov <contact@kchibisov.com>
@moko256 @kchibisov
fix some codes from review
31adbcd 
Co-authored-by: Kirill Chibisov <contact@kchibisov.com>
@moko256
run format
97eb1d7
kchibisov
kchibisov approved these changes Apr 11, 2022
View reviewed changes
Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

I can't test functional part of the PR, so leaving it to windows testers/maintainers.

@moko256
Copy link
Contributor Author

moko256 commented Apr 11, 2022

Thanks for reviewing!

@msiglreith msiglreith self-requested a review April 12, 2022 16:28
@moko256
Copy link
Contributor Author

moko256 commented Apr 13, 2022

@kchibisov Sorry, I changed CHANGELOG.md after your approves due to #2241 (review). Would you review this?

@kchibisov kchibisov merged commit bc90c3d into rust-windowing:composition-event Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@msiglreith msiglreith msiglreith approved these changes

@kchibisov kchibisov kchibisov approved these changes

Assignees
No one assigned
Labels
DS - windows
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@moko256 @msiglreith @kchibisov