Skip to content

Commit

Permalink
Fix some smaller broadcast bugs (#15993)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request

Resolves the following in #15812 

> - [x] `toggleBroadcastInput` isn't in the default settings
> - [x] The cursors forget to keep blinking if you focus each pane and
then unfocus them
> - [x] They don't stop blinking when you unbroadcast
> - [x] Broadcast border doesn't appear when you make new panes, but
they ARE broadcasted-to!

## References and Relevant Issues
x-ref:
* #2634
* #14393

## Detailed Description of the Pull Request / Additional comments

There was literally no logic in the original PR for starting the cursor
blinking. It's entirely unknowable how that ever worked. This makes it
all much more explicit.

We're taking the hacky `DisplayCursorWhileBlurred` from #15363, and
promoting that to the less-hacky `CursorVisibility`. Broadcast input
mode can use that to force the cursor to be visible always.


The last checkbox in that issue is harder, and I didn't want to further
pollute this delta with the paste plumbing.
  • Loading branch information
zadjii-msft authored Sep 19, 2023
1 parent 5f9624f commit f494d68
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 8 deletions.
4 changes: 4 additions & 0 deletions src/cascadia/TerminalApp/Pane.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2518,6 +2518,7 @@ std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> Pane::_Split(SplitDirect
_profile = nullptr;
_control = { nullptr };
_firstChild->_isDefTermSession = _isDefTermSession;
_firstChild->_broadcastEnabled = _broadcastEnabled;
}

_splitState = actualSplitType;
Expand Down Expand Up @@ -3173,6 +3174,9 @@ void Pane::EnableBroadcast(bool enabled)
if (_IsLeaf())
{
_broadcastEnabled = enabled;
_control.CursorVisibility(enabled ?
CursorDisplayState::Shown :
CursorDisplayState::Default);
UpdateVisuals();
}
else
Expand Down
15 changes: 15 additions & 0 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4698,6 +4698,21 @@ namespace winrt::TerminalApp::implementation
// the settings, change active panes, etc.
_activated = activated;
_updateThemeColors();

if (const auto& tab{ _GetFocusedTabImpl() })
{
if (tab->TabStatus().IsInputBroadcastActive())
{
tab->GetRootPane()->WalkTree([activated](const auto& p) {
if (const auto& control{ p->GetTerminalControl() })
{
control.CursorVisibility(activated ?
Microsoft::Terminal::Control::CursorDisplayState::Shown :
Microsoft::Terminal::Control::CursorDisplayState::Default);
}
});
}
}
}

winrt::fire_and_forget TerminalPage::_ControlCompletionsChangedHandler(const IInspectable sender,
Expand Down
47 changes: 44 additions & 3 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1051,8 +1051,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// _cursorTimer doesn't exist, and it would never turn on the
// cursor. To mitigate, we'll initialize the cursor's 'on' state
// with `_focused` here.
_core.CursorOn(_focused || DisplayCursorWhileBlurred);
if (DisplayCursorWhileBlurred)
_core.CursorOn(_focused || _displayCursorWhileBlurred());
if (_displayCursorWhileBlurred())
{
_cursorTimer->Start();
}
Expand Down Expand Up @@ -1968,7 +1968,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
TSFInputControl().NotifyFocusLeave();
}

if (_cursorTimer && !DisplayCursorWhileBlurred)
if (_cursorTimer && !_displayCursorWhileBlurred())
{
_cursorTimer->Stop();
_core.CursorOn(false);
Expand Down Expand Up @@ -3660,4 +3660,45 @@ namespace winrt::Microsoft::Terminal::Control::implementation
SelectionContextMenu().Hide();
_core.ContextMenuSelectOutput();
}

// Should the text cursor be displayed, even when the control isn't focused?
// n.b. "blur" is the opposite of "focus".
bool TermControl::_displayCursorWhileBlurred() const noexcept
{
return CursorVisibility() == Control::CursorDisplayState::Shown;
}
Control::CursorDisplayState TermControl::CursorVisibility() const noexcept
{
return _cursorVisibility;
}
void TermControl::CursorVisibility(Control::CursorDisplayState cursorVisibility)
{
_cursorVisibility = cursorVisibility;
if (!_initializedTerminal)
{
return;
}

if (_displayCursorWhileBlurred())
{
// If we should be ALWAYS displaying the cursor, turn it on and start blinking.
_core.CursorOn(true);
if (_cursorTimer.has_value())
{
_cursorTimer->Start();
}
}
else
{
// Otherwise, if we're unfocused, then turn the cursor off and stop
// blinking. (if we're focused, then we're already doing the right
// thing)
const auto focused = FocusState() != FocusState::Unfocused;
if (!focused && _cursorTimer.has_value())
{
_cursorTimer->Stop();
}
_core.CursorOn(focused);
}
}
}
9 changes: 6 additions & 3 deletions src/cascadia/TerminalControl/TermControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation
TerminalConnection::ITerminalConnection Connection();
void Connection(const TerminalConnection::ITerminalConnection& connection);

Control::CursorDisplayState CursorVisibility() const noexcept;
void CursorVisibility(Control::CursorDisplayState cursorVisibility);

// -------------------------------- WinRT Events ---------------------------------
// clang-format off
WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
Expand Down Expand Up @@ -194,9 +197,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation

WINRT_OBSERVABLE_PROPERTY(winrt::Windows::UI::Xaml::Media::Brush, BackgroundBrush, _PropertyChangedHandlers, nullptr);

public:
til::property<bool> DisplayCursorWhileBlurred{ false };

private:
friend struct TermControlT<TermControl>; // friend our parent so it can bind private event handlers

Expand Down Expand Up @@ -258,6 +258,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
Windows::Foundation::Collections::IObservableVector<Windows::UI::Xaml::Controls::ICommandBarElement> _originalSelectedPrimaryElements{ nullptr };
Windows::Foundation::Collections::IObservableVector<Windows::UI::Xaml::Controls::ICommandBarElement> _originalSelectedSecondaryElements{ nullptr };

Control::CursorDisplayState _cursorVisibility{ Control::CursorDisplayState::Default };

inline bool _IsClosing() const noexcept
{
#ifndef NDEBUG
Expand Down Expand Up @@ -376,6 +378,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

void _SelectCommandHandler(const IInspectable& sender, const IInspectable& args);
void _SelectOutputHandler(const IInspectable& sender, const IInspectable& args);
bool _displayCursorWhileBlurred() const noexcept;

struct Revokers
{
Expand Down
8 changes: 7 additions & 1 deletion src/cascadia/TerminalControl/TermControl.idl
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ import "ControlCore.idl";
namespace Microsoft.Terminal.Control
{

enum CursorDisplayState
{
Default,
Shown
};

[default_interface] runtimeclass TermControl : Windows.UI.Xaml.Controls.UserControl,
IDirectKeyListener,
IMouseWheelListener,
Expand Down Expand Up @@ -121,7 +127,7 @@ namespace Microsoft.Terminal.Control
// opacity set by the settings should call this instead.
Double BackgroundOpacity { get; };

Boolean DisplayCursorWhileBlurred;
CursorDisplayState CursorVisibility;

Windows.UI.Xaml.Media.Brush BackgroundBrush { get; };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
_previewControl = Control::TermControl(settings, settings, *_previewConnection);
_previewControl.IsEnabled(false);
_previewControl.AllowFocusWhenDisabled(false);
_previewControl.DisplayCursorWhileBlurred(true);
_previewControl.CursorVisibility(Microsoft::Terminal::Control::CursorDisplayState::Shown);
ControlPreview().Child(_previewControl);
}

Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/defaults.json
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@
{ "command": { "action": "swapPane", "direction": "previousInOrder"} },
{ "command": { "action": "swapPane", "direction": "nextInOrder"} },
{ "command": { "action": "swapPane", "direction": "first" } },
{ "command": "toggleBroadcastInput" },
{ "command": "togglePaneZoom" },
{ "command": "toggleSplitOrientation" },
{ "command": "toggleReadOnlyMode" },
Expand Down

0 comments on commit f494d68

Please sign in to comment.