Merged
Conversation
DHowett-MSFT
approved these changes
Feb 26, 2020
zadjii-msft
approved these changes
Feb 26, 2020
Member
zadjii-msft
left a comment
There was a problem hiding this comment.
I'm cool with this. I'll let you experiment with moving this block of code above the uiaEngine enablement like Dustin proposed before automerging though
carlos-zamora
approved these changes
Feb 26, 2020
|
Hello @leonMSFT! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
DHowett-MSFT
pushed a commit
that referenced
this pull request
Feb 28, 2020
This commit changes how we handle focus and tabstop-ness in TermControl and its descendent controls. The root of the issue that #4031 set out to investigate is that TermControl hosts a bunch of other controls, but it's using event tunneling to get first dibs on every bit of keyboard input sent its way -- even for things that live inside it. This required us to hack around and detemrine whether the input was intended for someone else before eventually ignoring it or using it. I believe the _correct_ thing for us to do is to direct user keyboard input to a control that lives logically "inside" the terminal surface and have that control hold focus and process keyboard events. Herein, we introduce an empty and invisible UserControl to hold focus inside the terminal surface. Everything I've tested works great here. It's important to note that the UserControl is invisible. Unfortunately, this means that it can't receive **mouse** input. Its parent, the SwapChainPanel acting as the terminal surface, can. I've gone through and unmarked things that should not show up in the tabstop list and changed the focus parameters for things that should. Doing this lets us kill TermControl's dependency on all private details in SearchBoxControl, including the bit where we could ask "do you think you should be focused?", by letting Xaml handle focus. We now listen to more focus events and funnel focus where we think it belongs. For SearchBoxControl, that means we focus the text field and continue to select all the text in it when the SearchBoxControl gets focus. Fixes #4031. This is an alternative solution to #4723, so I reverted that fix.
3 tasks
DHowett-MSFT
pushed a commit
that referenced
this pull request
Feb 28, 2020
This commit changes how we handle focus and tabstop-ness in TermControl and its descendent controls. The root of the issue that #4031 set out to investigate is that TermControl hosts a bunch of other controls, but it's using event tunneling to get first dibs on every bit of keyboard input sent its way -- even for things that live inside it. This required us to hack around and detemrine whether the input was intended for someone else before eventually ignoring it or using it. I believe the _correct_ thing for us to do is to direct user keyboard input to a control that lives logically "inside" the terminal surface and have that control hold focus and process keyboard events. Herein, we introduce an empty and invisible UserControl to hold focus inside the terminal surface. Everything I've tested works great here. It's important to note that the UserControl is invisible. Unfortunately, this means that it can't receive **mouse** input. Its parent, the SwapChainPanel acting as the terminal surface, can. I've gone through and unmarked things that should not show up in the tabstop list and changed the focus parameters for things that should. Doing this lets us kill TermControl's dependency on all private details in SearchBoxControl, including the bit where we could ask "do you think you should be focused?", by letting Xaml handle focus. We now listen to more focus events and funnel focus where we think it belongs. For SearchBoxControl, that means we focus the text field and continue to select all the text in it when the SearchBoxControl gets focus. Fixes #4031. This is an alternative solution to #4723, so I reverted that fix.
This pull request was closed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of the Pull Request
Currently, when the user attempts to type using IME while the Search Box is focused, the input goes to the terminal instead. This is due to the fact that the
TSFInputControlassumes it's in control whenever TermControl gets focus. So, it'll intercept IME input before the Search Box receives it. We simply need to modifyTermControl::GotFocusto check if the SearchBox has focus. If it does,TSFInputControl::NotifyFocusEntershouldn't be called.As a small side fix, I've also disabled the terminal cursor blinking when the Search Box has focus.
Thinking a little further, if we have more features in the future that behave like search box (i.e. advanced tab switcher, or any other XAML controls that pop up) and require text input, we might need to create a sort of "AnyOtherTextControlInFocus" function to see if TSFInputControl should receive focus or not.
PR Checklist
Validation Steps Performed
Search works as expected with IME. Composition picker appears underneath Search Box when typing IME in the Search Box. Clicking outside of the Search Box still returns control to TSFInputControl/TermControl.
Terminal Cursor blinks when it has focus, and doesn't when the Search Box has focus.