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

Tunnel F7 keypresses directly into special handlers in TermControl #4807

Merged
3 commits merged into from Mar 5, 2020

Conversation

DHowett-MSFT
Copy link
Contributor

The Xaml input stack doesn't allow an application to suppress the "caret
browsing" dialog experience triggered when you press F7.

The official recommendation from the Xaml team is to catch F7 before we
hand it off.

This commit introduces a special F7 handler and an ad-hoc implementation of event bubbling.
Runtime classes implementing a custom IF7Listener interface are
considered during a modified focus parent walk to determine who can
handle F7 specifically.

If the recipient control handles F7, we suppress the message completely.

This event bubbler has some minor issues -- the search box will not be
able to receive F7 because its parent control implements the handler.
Since search is already mostly a text box, it doesn't need special
caret browsing functionality as far as I can tell.

TermControl implements its OnF7Pressed handler by synthesizing a
keybindings event and an event to feed into Terminal Core directly.

It's not possible to create a synthetic KeyPressRoutedEvent; if it were,
I would have just popped one into the traditional input queue. :)

Fixes #638.

The Xaml input stack doesn't allow an application to suppress the "caret
browsing" dialog experience triggered when you press F7.

The official recommendation from the Xaml team is to catch F7 before we
hand it off.

This commit introduces a special F7 handler and an ad-hoc implementation of event bubbling.
Runtime classes implementing a custom IF7Listener interface are
considered during a modified focus parent walk to determine who can
handle F7 specifically.

If the recipient control handles F7, we suppress the message completely.

This event bubbler has some minor issues -- the search box will not be
able to receive F7 because its parent control implements the handler.
Since search is already mostly a text box, it doesn't _need_ special
caret browsing functionality as far as I can tell.

TermControl implements its OnF7Pressed handler by synthesizing a
keybindings event and an event to feed into Terminal Core directly.

It's not possible to create a synthetic KeyPressRoutedEvent; if it were,
I would have just popped one into the traditional input queue. :)

Fixes #638.
@DHowett-MSFT
Copy link
Contributor Author

It even works with modifiers!
Here's me sending WSL F7 with a bunch of mods.

image

src/cascadia/TerminalControl/TermControl.idl Show resolved Hide resolved
@@ -7,6 +7,14 @@ namespace Microsoft.Terminal.TerminalControl
delegate void FontSizeChangedEventArgs(Int32 width, Int32 height, Boolean isInitialChange);
delegate void ScrollPositionChangedEventArgs(Int32 viewTop, Int32 viewHeight, Int32 bufferLength);

// c++/winrt makes it difficult to share this idl between two projects,
Copy link
Member

Choose a reason for hiding this comment

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

Is there any reason we don't just define this in the TerminalControl project, and just reference it as Microsoft.TerminalControl.IF7Listener in TerminalApp?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I .. honestly don't know why I didn't choose that. It seems wrong for WindowsTerminal.exe to be depending on an interface from all the way down in TerminalControl; it's an app concern, and Control just happens to share the same concern.

src/cascadia/TerminalControl/TermControl.cpp Show resolved Hide resolved
src/cascadia/WindowsTerminal/main.cpp Show resolved Hide resolved
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

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

Mainly just wondering if we can/should abstract this further away from just F7. But if caret browsing is a SUPER unique scenario, then whatever. Not worth blocking though :)

src/cascadia/TerminalApp/IF7Listener.idl Show resolved Hide resolved
src/cascadia/TerminalApp/IF7Listener.idl Show resolved Hide resolved
@DHowett-MSFT DHowett-MSFT added the AutoMerge Marked for automatic merge by the bot when requirements are met label Mar 5, 2020
@ghost
Copy link

ghost commented Mar 5, 2020

Hello @DHowett-MSFT!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

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 (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit a3d68d2 into master Mar 5, 2020
@ghost ghost deleted the dev/duhowtt/f7 branch March 5, 2020 20:35
@luzhkovvv luzhkovvv mentioned this pull request Aug 14, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AutoMerge Marked for automatic merge by the bot when requirements are met
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pressing F7 in Terminal running CMD brings up both the history AND a caret browsing message
4 participants