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

Add a setting to flash the pane when BEL is emitted #9270

Merged
32 commits merged into from
May 24, 2021
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
370e0c0
initial
PankajBhojwani Feb 18, 2021
3d7c9db
optional timer
PankajBhojwani Feb 18, 2021
54a683d
add setting
PankajBhojwani Feb 19, 2021
b4ae30c
Merge branch 'main' of https://github.com/microsoft/terminal into dev…
PankajBhojwani Feb 19, 2021
d59a432
change timer
PankajBhojwani Feb 24, 2021
c1172cd
format
PankajBhojwani Mar 1, 2021
e0c9f89
stash, lock
PankajBhojwani Mar 19, 2021
e89f273
weak this
PankajBhojwani Mar 19, 2021
f93c9e8
Merge branch 'main' of https://github.com/microsoft/terminal into dev…
PankajBhojwani Mar 24, 2021
4c125c3
resw, schema
PankajBhojwani Mar 24, 2021
4b19b57
merge main, fix conflicts
PankajBhojwani May 7, 2021
76c182a
use xaml light
PankajBhojwani May 7, 2021
5ed7e93
spell
PankajBhojwani May 7, 2021
678cfb5
er what
PankajBhojwani May 7, 2021
cecc6d8
Merge branch 'main' of https://github.com/microsoft/terminal into dev…
PankajBhojwani May 7, 2021
23dc1fd
move light to own file
PankajBhojwani May 10, 2021
9282a16
nits
PankajBhojwani May 10, 2021
eedb317
lazy load
PankajBhojwani May 10, 2021
5b72726
some cleanup
PankajBhojwani May 10, 2021
692ad14
animation
PankajBhojwani May 12, 2021
8a42804
Format
PankajBhojwani May 12, 2021
b3c50f8
small changes
PankajBhojwani May 13, 2021
ab0ce5f
ahhh
PankajBhojwani May 17, 2021
8026698
deprecate visual
PankajBhojwani May 19, 2021
fb27787
here and there
PankajBhojwani May 19, 2021
2f9bc61
spell
PankajBhojwani May 19, 2021
084ff79
fix crash, two-way binding
PankajBhojwani May 20, 2021
f1237d9
remove flag map
PankajBhojwani May 20, 2021
b675b2a
Cleanup
PankajBhojwani May 21, 2021
6c9d8ef
== flag
PankajBhojwani May 21, 2021
730cf09
Ternary
PankajBhojwani May 24, 2021
f5bb54e
Apply suggestions from code review
DHowett May 24, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@
"type": "string",
"enum": [
"audible",
"visual"
"window",
"taskbar"
]
}
},
Expand All @@ -46,6 +47,8 @@
"enum": [
"audible",
"visual",
"taskbar",
"window",
"all",
"none"
]
Expand Down Expand Up @@ -1171,7 +1174,7 @@
},
"bellStyle": {
"default": "audible",
"description": "Controls what happens when the application emits a BEL character. When set to \"all\", the Terminal will play a sound and flash the taskbar icon. An array of specific behaviors can also be used. Supported array values include `audible` and `visual`. When set to \"none\", nothing will happen.",
"description": "Controls what happens when the application emits a BEL character. When set to \"all\", the Terminal will play a sound, flash the taskbar icon (if the terminal window is not in focus) and flash the window. An array of specific behaviors can also be used. Supported array values include `audible`, `window` and `taskbar`. When set to \"none\", nothing will happen.",
"$ref": "#/definitions/BellStyle"
},
"closeOnExit": {
Expand Down
9 changes: 7 additions & 2 deletions src/cascadia/TerminalApp/Pane.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -378,8 +378,13 @@ void Pane::_ControlWarningBellHandler(const winrt::Windows::Foundation::IInspect
PlaySound(soundAlias, NULL, SND_ALIAS_ID | SND_ASYNC | SND_SENTRY);
}

// raise the event with the bool value corresponding to the visual flag
_PaneRaiseBellHandlers(nullptr, WI_IsFlagSet(paneProfile.BellStyle(), winrt::Microsoft::Terminal::Settings::Model::BellStyle::Visual));
if (WI_IsFlagSet(paneProfile.BellStyle(), winrt::Microsoft::Terminal::Settings::Model::BellStyle::Window))
{
_control.BellLightOn();
Copy link
Member

Choose a reason for hiding this comment

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

this seems like the wrong abstraction-- the app should not tell the control to blink after the control tells the app that there was a bell.

Copy link
Member

Choose a reason for hiding this comment

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

Your comment about the visual bell moving up into the application resonates with me. Thank you.

}

// raise the event with the bool value corresponding to the taskbar flag
_PaneRaiseBellHandlers(nullptr, WI_IsFlagSet(paneProfile.BellStyle(), winrt::Microsoft::Terminal::Settings::Model::BellStyle::Taskbar));
}
}
}
Expand Down
94 changes: 94 additions & 0 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "../../types/inc/Utils.hpp"

#include "TermControl.g.cpp"
#include "VisualBellLight.g.cpp"
#include "TermControlAutomationPeer.h"

using namespace ::Microsoft::Console::Types;
Expand Down Expand Up @@ -58,6 +59,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_lastAutoScrollUpdateTime{ std::nullopt },
_cursorTimer{},
_blinkTimer{},
_bellLightTimer{},
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved
_searchBox{ nullptr }
{
InitializeComponent();
Expand Down Expand Up @@ -2358,6 +2360,34 @@ namespace winrt::Microsoft::Terminal::Control::implementation
return _core->TaskbarProgress();
}

void TermControl::BellLightOn()
{
if (!_bellLightTimer)
{
// Start the timer, when the timer ticks we switch off the light
DispatcherTimer invertTimer;
invertTimer.Interval(std::chrono::milliseconds(2000));
invertTimer.Tick({ get_weak(), &TermControl::_BellLightOff });
invertTimer.Start();
_bellLightTimer.emplace(std::move(invertTimer));

// Switch on the light
VisualBellLight::SetIsTarget(RootGrid(), true);
}
}

void TermControl::_BellLightOff(Windows::Foundation::IInspectable const& /* sender */,
Windows::Foundation::IInspectable const& /* e */)
{
if (_bellLightTimer && !_closing)
lhecker marked this conversation as resolved.
Show resolved Hide resolved
{
// Stop the timer and switch off the light
_bellLightTimer.value().Stop();
_bellLightTimer = std::nullopt;
VisualBellLight::SetIsTarget(RootGrid(), false);
}
}

// Method Description:
// - Checks whether the control is in a read-only mode (in this mode node input is sent to connection).
// Return Value:
Expand Down Expand Up @@ -2492,4 +2522,68 @@ namespace winrt::Microsoft::Terminal::Control::implementation
{
_playWarningBell->Run();
}

Windows::UI::Xaml::DependencyProperty VisualBellLight::m_isTargetProperty =
Windows::UI::Xaml::DependencyProperty::RegisterAttached(
L"IsTarget",
winrt::xaml_typename<bool>(),
winrt::xaml_typename<winrt::Microsoft::Terminal::Control::VisualBellLight>(),
Windows::UI::Xaml::PropertyMetadata{ winrt::box_value(false), Windows::UI::Xaml::PropertyChangedCallback{ &VisualBellLight::OnIsTargetChanged } });
Copy link
Member

Choose a reason for hiding this comment

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

Check out...

  • https://github.com/microsoft/terminal/blob/main/src/cascadia/TerminalSettingsEditor/SettingContainer.h
    • there's a DEPENDENCY_PROPERTY macro I created there. You may be able to move this into cppwinrt_utils and use it yourself.
  • DependencyProperty SettingContainer::_HeaderProperty{ nullptr };
    DependencyProperty SettingContainer::_HelpTextProperty{ nullptr };
    DependencyProperty SettingContainer::_HasSettingValueProperty{ nullptr };
    DependencyProperty SettingContainer::_SettingOverrideSourceProperty{ nullptr };
    SettingContainer::SettingContainer()
    {
    _InitializeProperties();
    }
    void SettingContainer::_InitializeProperties()
    {
    // Initialize any SettingContainer dependency properties here.
    // This performs a lazy load on these properties, instead of
    // initializing them when the DLL loads.
    if (!_HeaderProperty)
    {
    _HeaderProperty =
    DependencyProperty::Register(
    L"Header",
    xaml_typename<IInspectable>(),
    xaml_typename<Editor::SettingContainer>(),
    PropertyMetadata{ nullptr });
    }
    if (!_HelpTextProperty)
    {
    _HelpTextProperty =
    DependencyProperty::Register(
    L"HelpText",
    xaml_typename<hstring>(),
    xaml_typename<Editor::SettingContainer>(),
    PropertyMetadata{ box_value(L"") });
    }
    if (!_HasSettingValueProperty)
    {
    _HasSettingValueProperty =
    DependencyProperty::Register(
    L"HasSettingValue",
    xaml_typename<bool>(),
    xaml_typename<Editor::SettingContainer>(),
    PropertyMetadata{ box_value(false), PropertyChangedCallback{ &SettingContainer::_OnHasSettingValueChanged } });
    }
    if (!_SettingOverrideSourceProperty)
    {
    _SettingOverrideSourceProperty =
    DependencyProperty::Register(
    L"SettingOverrideSource",
    xaml_typename<bool>(),
    xaml_typename<Editor::SettingContainer>(),
    PropertyMetadata{ nullptr });
    }
    }
    • We lazy load the dependency properties in _InitializeProperties. I'd take a look there and apply the same code pattern here.

Copy link
Contributor Author

@PankajBhojwani PankajBhojwani May 10, 2021

Choose a reason for hiding this comment

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

I think we'll probably need a different macro for this one since it involves getting/setting the property on a target (and I think we can wait on adding those until we actually want more lights in the code) - done the lazy load though!


void VisualBellLight::OnConnected(Windows::UI::Xaml::UIElement const& /* newElement */)
{
if (!CompositionLight())
{
// OnConnected is called when the first target UIElement is shown on the screen. This enables delaying composition object creation until it's actually necessary.
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved
auto spotLight2{ Windows::UI::Xaml::Window::Current().Compositor().CreateAmbientLight() };
spotLight2.Color(Windows::UI::Colors::WhiteSmoke());
spotLight2.Intensity(static_cast<float>(1.5));
CompositionLight(spotLight2);
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved
}
}

void VisualBellLight::OnDisconnected(Windows::UI::Xaml::UIElement const& /* oldElement */)
{
// OnDisconnected is called when there are no more target UIElements on the screen.
// Dispose of composition resources when no longer in use.
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved
if (CompositionLight())
{
CompositionLight(nullptr);
}
}

winrt::hstring VisualBellLight::GetId()
{
return VisualBellLight::GetIdStatic();
}

void VisualBellLight::OnIsTargetChanged(Windows::UI::Xaml::DependencyObject const& d, Windows::UI::Xaml::DependencyPropertyChangedEventArgs const& e)
{
auto uielem{ d.try_as<Windows::UI::Xaml::UIElement>() };
auto brush{ d.try_as<Windows::UI::Xaml::Media::Brush>() };
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved

auto isAdding = winrt::unbox_value<bool>(e.NewValue());
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved
if (isAdding)
{
if (uielem)
{
Windows::UI::Xaml::Media::XamlLight::AddTargetElement(VisualBellLight::GetIdStatic(), uielem);
}
else if (brush)
{
Windows::UI::Xaml::Media::XamlLight::AddTargetBrush(VisualBellLight::GetIdStatic(), brush);
}
}
else
{
if (uielem)
{
Windows::UI::Xaml::Media::XamlLight::RemoveTargetElement(VisualBellLight::GetIdStatic(), uielem);
}
else if (brush)
{
Windows::UI::Xaml::Media::XamlLight::RemoveTargetBrush(VisualBellLight::GetIdStatic(), brush);
}
}
}
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved
}
40 changes: 40 additions & 0 deletions src/cascadia/TerminalControl/TermControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#pragma once

#include "TermControl.g.h"
#include "VisualBellLight.g.h"
#include "EventArgs.h"
#include "../../renderer/base/Renderer.hpp"
#include "../../renderer/dx/DxRenderer.hpp"
Expand Down Expand Up @@ -98,6 +99,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
const winrt::hstring& padding,
const uint32_t dpi);

void BellLightOn();

bool ReadOnly() const noexcept;
void ToggleReadOnly();

Expand Down Expand Up @@ -129,6 +132,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

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

winrt::com_ptr<ControlCore> _core;
winrt::com_ptr<ControlInteractivity> _interactivity;
Expand Down Expand Up @@ -166,6 +170,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

std::optional<Windows::UI::Xaml::DispatcherTimer> _cursorTimer;
std::optional<Windows::UI::Xaml::DispatcherTimer> _blinkTimer;
std::optional<Windows::UI::Xaml::DispatcherTimer> _bellLightTimer;

event_token _coreOutputEventToken;

Expand Down Expand Up @@ -202,6 +207,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

void _CursorTimerTick(Windows::Foundation::IInspectable const& sender, Windows::Foundation::IInspectable const& e);
void _BlinkTimerTick(Windows::Foundation::IInspectable const& sender, Windows::Foundation::IInspectable const& e);
void _BellLightOff(Windows::Foundation::IInspectable const& sender, Windows::Foundation::IInspectable const& e);

void _SetEndSelectionPointAtCursor(Windows::Foundation::Point const& cursorPosition);

Expand Down Expand Up @@ -249,9 +255,43 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void _coreRaisedNotice(const IInspectable& s, const Control::NoticeEventArgs& args);
void _coreWarningBell(const IInspectable& sender, const IInspectable& args);
};

struct VisualBellLight : VisualBellLightT<VisualBellLight>
{
VisualBellLight() = default;

winrt::hstring GetId();

static Windows::UI::Xaml::DependencyProperty IsTargetProperty() { return m_isTargetProperty; }

static bool GetIsTarget(Windows::UI::Xaml::DependencyObject const& target)
{
return winrt::unbox_value<bool>(target.GetValue(m_isTargetProperty));
}

static void SetIsTarget(Windows::UI::Xaml::DependencyObject const& target, bool value)
{
target.SetValue(m_isTargetProperty, winrt::box_value(value));
}

void OnConnected(Windows::UI::Xaml::UIElement const& newElement);
void OnDisconnected(Windows::UI::Xaml::UIElement const& oldElement);

static void OnIsTargetChanged(Windows::UI::Xaml::DependencyObject const& d, Windows::UI::Xaml::DependencyPropertyChangedEventArgs const& e);

inline static winrt::hstring GetIdStatic()
{
// This specifies the unique name of the light. In most cases you should use the type's full name.
return winrt::xaml_typename<winrt::Microsoft::Terminal::Control::VisualBellLight>().Name;
}

private:
static Windows::UI::Xaml::DependencyProperty m_isTargetProperty;
};
}

namespace winrt::Microsoft::Terminal::Control::factory_implementation
{
BASIC_FACTORY(TermControl);
BASIC_FACTORY(VisualBellLight);
}
10 changes: 10 additions & 0 deletions src/cascadia/TerminalControl/TermControl.idl
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,17 @@ namespace Microsoft.Terminal.Control
void ToggleShaderEffects();
void SendInput(String input);

void BellLightOn();

Boolean ReadOnly { get; };
void ToggleReadOnly();
}

[default_interface] runtimeclass VisualBellLight : Windows.UI.Xaml.Media.XamlLight
{
VisualBellLight();
static Windows.UI.Xaml.DependencyProperty IsTargetProperty { get; };
static Boolean GetIsTarget(Windows.UI.Xaml.DependencyObject target);
static void SetIsTarget(Windows.UI.Xaml.DependencyObject target, Boolean value);
}
}
3 changes: 3 additions & 0 deletions src/cascadia/TerminalControl/TermControl.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
-->

<Grid x:Name="RootGrid">
<Grid.Lights>
<local:VisualBellLight />
</Grid.Lights>
<Image x:Name="BackgroundImage"
AutomationProperties.AccessibilityView="Raw" />
<Grid>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -899,9 +899,13 @@
<value>All</value>
<comment>An option to choose from for the "bell style" setting. When selected, a combination of the other bell styles is used to notify the user.</comment>
</data>
<data name="Profile_BellStyleVisual.Content" xml:space="preserve">
<value>Visual (flash taskbar)</value>
<comment>An option to choose from for the "bell style" setting. When selected, a visual notification is used to notify the user. In this case, the taskbar is flashed.</comment>
PankajBhojwani marked this conversation as resolved.
Show resolved Hide resolved
<data name="Profile_BellStyleTaskbar.Content" xml:space="preserve">
<value>Flash taskbar</value>
<comment>An option to choose from for the "bell style" setting. When selected, a visual notification is used to notify the user. In this case, the taskbar is flashed (only if the terminal window is not in focus).</comment>
</data>
<data name="Profile_BellStyleWindow.Content" xml:space="preserve">
<value>Flash window</value>
<comment>An option to choose from for the "bell style" setting. When selected, a visual notification is used to notify the user. In this case, the window is flashed.</comment>
</data>
<data name="ColorScheme_AddNewButton.Text" xml:space="preserve">
<value>Add new</value>
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalSettingsModel/Profile.idl
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ namespace Microsoft.Terminal.Settings.Model
enum BellStyle
{
DHowett marked this conversation as resolved.
Show resolved Hide resolved
Audible = 0x1,
Visual = 0x2,
DHowett marked this conversation as resolved.
Show resolved Hide resolved
Window = 0x2,
DHowett marked this conversation as resolved.
Show resolved Hide resolved
Taskbar = 0x4,
All = 0xffffffff
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,11 @@ JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Control::ScrollbarState)

JSON_FLAG_MAPPER(::winrt::Microsoft::Terminal::Settings::Model::BellStyle)
{
static constexpr std::array<pair_type, 4> mappings = {
static constexpr std::array<pair_type, 5> mappings = {
pair_type{ "none", AllClear },
pair_type{ "audible", ValueType::Audible },
pair_type{ "visual", ValueType::Visual },
Copy link
Member

Choose a reason for hiding this comment

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

If you want to fake deprecate "visual", you'll probably want to keep this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep! Though it now maps to Window | Taskbar

pair_type{ "window", ValueType::Window },
pair_type{ "taskbar", ValueType::Taskbar },
pair_type{ "all", AllSet },
};

Expand Down