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

TSFInputControl NotifyFocusLeave causes IME to no longer show #3645

Closed
philnach opened this issue Nov 20, 2019 · 1 comment · Fixed by #4140
Closed

TSFInputControl NotifyFocusLeave causes IME to no longer show #3645

philnach opened this issue Nov 20, 2019 · 1 comment · Fixed by #4140
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone

Comments

@philnach
Copy link
Member

from #1919

Code TODO, if the IME is notified that Focus has left it causes the IME to no longer show. It's unclear if the IME needs to know Focus is leaving. Investigate if the call is required. and re-enable the call in the code:

    void TSFInputControl::NotifyFocusLeave()
    {
        if (_editContext != nullptr)
        {
            // _editContext.NotifyFocusLeave();
        }
    }
@philnach philnach added Area-Input Related to input processing (key presses, mouse, etc.) Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. labels Nov 20, 2019
@ghost ghost added the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Nov 20, 2019
@DHowett-MSFT DHowett-MSFT added this to the Terminal v1.0 milestone Nov 21, 2019
@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Nov 21, 2019
@ghost ghost added the In-PR This issue has a related PR label Jan 7, 2020
@ghost ghost closed this as completed in #4140 Jan 8, 2020
ghost pushed a commit that referenced this issue Jan 8, 2020
The first argument to `NotifyTextChanged` incorrectly was `[0,0]`
instead of the length of the text to be removed from the
`CoreTextEditContext`.

Best source of documentation for `NotifyTextChanged`:
https://docs.microsoft.com/en-us/windows/uwp/design/input/custom-text-input#overriding-text-updates

FYI @DHowett-MSFT (just in case): C++/WinRT uses `winrt::param::hstring`
for string parameters which intelligently borrows strings. As such you
can simply pass a `std::wstring` to most WinRT methods without the need
of having to allocate an intermediate `hstring`. 🙂

## Validation Steps Performed

I followed the reproduction instructions of #3706 and #3745 and ensured
the issue doesn't happen anymore.

Closes #3645
Closes #3706
Closes #3745
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Jan 8, 2020
@ghost
Copy link

ghost commented Jan 14, 2020

🎉This issue was addressed in #4140, which has now been successfully released as Windows Terminal Preview v0.8.10091.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants