Raise ShortcutActions with the sender (tab, control) context#15773
Merged
zadjii-msft merged 14 commits intomainfrom Aug 24, 2023
Merged
Raise ShortcutActions with the sender (tab, control) context#15773zadjii-msft merged 14 commits intomainfrom
zadjii-msft merged 14 commits intomainfrom
Conversation
…d the event, as a parameter to ShortcutActionDispatch::DoAction
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
DHowett
approved these changes
Aug 10, 2023
Member
DHowett
left a comment
There was a problem hiding this comment.
Third thought about using actions: actually, I like it even more than I expected. It removes the handlers dance from Page.
Comment on lines
+534
to
+535
| winrt::Microsoft::Terminal::Control::TermControl _senderOrActiveControl(const winrt::Windows::Foundation::IInspectable& sender); | ||
| winrt::com_ptr<TerminalTab> _senderOrFocusedTab(const IInspectable& sender); |
Member
There was a problem hiding this comment.
For the future: technically, we might want the concept of a "stack" of senders. A message coming from a control has an associated pane and tab and eventually, window. Those things might matter to different recipients, or different action types.
lhecker
approved these changes
Aug 23, 2023
Member
lhecker
left a comment
There was a problem hiding this comment.
Exactly what Dustin said. LGTM though.
…ispatch-with-sender
4 tasks
DHowett
approved these changes
Aug 24, 2023
DHowett
pushed a commit
that referenced
this pull request
Sep 22, 2023
This PR's goal is to allow something like a `Tab` to raise a ShortcutAction, by saying "this action should be performed on ME". We've had a whole category of these issues in the past: * #15734 * #15760 * #13579 * #13942 * #13942 * Heck even dating back to #10832 So, this tries to remove a bit of that footgun. This probably isn't the _final_ form of what this refactor might look like, but the code is certainly better than before. Basically, there's a few bits: * `ShortcutActionDispatch.DoAction` now takes a `sender`, which can be _anything_. * Most actions that use a "Get the focused _thing_ then do something to it" are changed to "If there was a sender, let's use that - otherwise, we'll use the focused _thing_". * TerminalTab was largely refactored to use this, instead of making requests to the `TerminalPage` to just do a thing to it. I've got a few TODO!s left, but wanted to get initial feedback. * [x] `TerminalPage::_HandleTogglePaneZoom` * [x] `TerminalPage::_HandleFocusPane` * [x] `TerminalPage::_MoveTab` Closes #15734 (cherry picked from commit 30eb9ee) Service-Card-Id: 90438493 Service-Version: 1.18
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
3 participants
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.
This PR's goal is to allow something like a
Tabto raise a ShortcutAction, by saying "this action should be performed on ME". We've had a whole category of these issues in the past:So, this tries to remove a bit of that footgun. This probably isn't the final form of what this refactor might look like, but the code is certainly better than before.
Basically, there's a few bits:
ShortcutActionDispatch.DoActionnow takes asender, which can be anything.TerminalPageto just do a thing to it.I've got a few TODO!s left, but wanted to get initial feedback.
TerminalPage::_HandleTogglePaneZoomTerminalPage::_HandleFocusPaneTerminalPage::_MoveTabCloses #15734