feedback from PR

src/cascadia/TerminalApp/AppLogic.cpp

@@ -1121,6 +1121,16 @@ namespace winrt::TerminalApp::implementation
         return RS_(L"ApplicationVersionUnknown");
     }
 
+    bool AppLogic::FocusMode() const
+    {
+        return _root ? _root->FocusMode() : false;
+    }
+
+    bool AppLogic::Fullscreen() const
+    {
+        return _root ? _root->Fullscreen() : false;
+    }
+
     bool AppLogic::AlwaysOnTop() const
     {
         return _root ? _root->AlwaysOnTop() : false;

src/cascadia/TerminalApp/AppLogic.h

@@ -33,6 +33,8 @@ namespace winrt::TerminalApp::implementation
 
         winrt::hstring ApplicationDisplayName() const;
         winrt::hstring ApplicationVersion() const;
+        bool FocusMode() const;
+        bool Fullscreen() const;
         bool AlwaysOnTop() const;
 
         Windows::Foundation::Point GetLaunchDimensions(uint32_t dpi);
@@ -105,9 +107,9 @@ namespace winrt::TerminalApp::implementation
         FORWARDED_TYPED_EVENT(SetTitleBarContent, winrt::Windows::Foundation::IInspectable, winrt::Windows::UI::Xaml::UIElement, _root, SetTitleBarContent);
         FORWARDED_TYPED_EVENT(TitleChanged, winrt::Windows::Foundation::IInspectable, winrt::hstring, _root, TitleChanged);
         FORWARDED_TYPED_EVENT(LastTabClosed, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::LastTabClosedEventArgs, _root, LastTabClosed);
-        FORWARDED_TYPED_EVENT(ToggleFocusMode, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::ToggleFocusModeEventArgs, _root, ToggleFocusMode);
-        FORWARDED_TYPED_EVENT(ToggleFullscreen, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::ToggleFullscreenEventArgs, _root, ToggleFullscreen);
-        FORWARDED_TYPED_EVENT(AlwaysOnTopChanged, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::AlwaysOnTopChangedEventArgs, _root, AlwaysOnTopChanged);
+        FORWARDED_TYPED_EVENT(FocusModeChanged, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable, _root, FocusModeChanged);
+        FORWARDED_TYPED_EVENT(FullscreenChanged, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable, _root, FullscreenChanged);
+        FORWARDED_TYPED_EVENT(AlwaysOnTopChanged, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable, _root, AlwaysOnTopChanged);
     };
 }
 

src/cascadia/TerminalApp/AppLogic.idl

@@ -41,6 +41,8 @@ namespace TerminalApp
         String ApplicationDisplayName { get; };
         String ApplicationVersion { get; };
 
+        Boolean FocusMode { get; };
+        Boolean Fullscreen { get; };
         Boolean AlwaysOnTop { get; };
 
         Windows.Foundation.Point GetLaunchDimensions(UInt32 dpi);
@@ -60,8 +62,8 @@ namespace TerminalApp
         event Windows.Foundation.TypedEventHandler<Object, String> TitleChanged;
         event Windows.Foundation.TypedEventHandler<Object, LastTabClosedEventArgs> LastTabClosed;
         event Windows.Foundation.TypedEventHandler<Object, Windows.UI.Xaml.ElementTheme> RequestedThemeChanged;
-        event Windows.Foundation.TypedEventHandler<Object, ToggleFocusModeEventArgs> ToggleFocusMode;
-        event Windows.Foundation.TypedEventHandler<Object, ToggleFullscreenEventArgs> ToggleFullscreen;
-        event Windows.Foundation.TypedEventHandler<Object, AlwaysOnTopChangedEventArgs> AlwaysOnTopChanged;
+        event Windows.Foundation.TypedEventHandler<Object, Object> FocusModeChanged;
+        event Windows.Foundation.TypedEventHandler<Object, Object> FullscreenChanged;
+        event Windows.Foundation.TypedEventHandler<Object, Object> AlwaysOnTopChanged;
     }
 }

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -1918,7 +1918,7 @@ namespace winrt::TerminalApp::implementation
         // Reload the current value of alwaysOnTop from the settings file. This
         // will let the user hot-reload this setting, but any runtime changes to
         // the alwaysOnTop setting will be lost.
-        _isAlwayOnTop = _settings->GlobalSettings().AlwaysOnTop();
+        _isAlwaysOnTop = _settings->GlobalSettings().AlwaysOnTop();
         _alwaysOnTopChangedHandlers(*this, nullptr);
     }
 
@@ -1976,30 +1976,30 @@ namespace winrt::TerminalApp::implementation
 
     // Method Description:
     // - Toggles borderless mode. Hides the tab row, and raises our
-    //   ToggleFocusMode event.
+    //   FocusModeChanged event.
     // Arguments:
     // - <none>
     // Return Value:
     // - <none>
     void TerminalPage::ToggleFocusMode()
     {
         _isInFocusMode = !_isInFocusMode;
-        _toggleFocusModeHandlers(*this, nullptr);
         _UpdateTabView();
+        _focusModeChangedHandlers(*this, nullptr);
     }
 
     // Method Description:
     // - Toggles fullscreen mode. Hides the tab row, and raises our
-    //   ToggleFullscreen event.
+    //   FullscreenChanged event.
     // Arguments:
     // - <none>
     // Return Value:
     // - <none>
     void TerminalPage::ToggleFullscreen()
     {
         _isFullscreen = !_isFullscreen;
-        _toggleFullscreenHandlers(*this, nullptr);
         _UpdateTabView();
+        _fullscreenChangedHandlers(*this, nullptr);
     }
 
     // Method Description:
@@ -2010,7 +2010,7 @@ namespace winrt::TerminalApp::implementation
     // - <none>
     void TerminalPage::ToggleAlwaysOnTop()
     {
-        _isAlwayOnTop = !_isAlwayOnTop;
+        _isAlwaysOnTop = !_isAlwaysOnTop;
         _alwaysOnTopChangedHandlers(*this, nullptr);
     }
 
@@ -2202,6 +2202,15 @@ namespace winrt::TerminalApp::implementation
         }
     }
 
+    bool TerminalPage::FocusMode() const
+    {
+        return _isInFocusMode;
+    }
+
+    bool TerminalPage::Fullscreen() const
+    {
+        return _isFullscreen;
+    }
     // Method Description:
     // - Returns true if we're currently in "Always on top" mode. When we're in
     //   always on top mode, the window should be on top of all other windows.
@@ -2213,7 +2222,7 @@ namespace winrt::TerminalApp::implementation
     // - true if we should be in "always on top" mode
     bool TerminalPage::AlwaysOnTop() const
     {
-        return _isAlwayOnTop;
+        return _isAlwaysOnTop;
     }
 
     // -------------------------------- WinRT Events ---------------------------------
@@ -2222,7 +2231,7 @@ namespace winrt::TerminalApp::implementation
     DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, TitleChanged, _titleChangeHandlers, winrt::Windows::Foundation::IInspectable, winrt::hstring);
     DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, LastTabClosed, _lastTabClosedHandlers, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::LastTabClosedEventArgs);
     DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, SetTitleBarContent, _setTitleBarContentHandlers, winrt::Windows::Foundation::IInspectable, UIElement);
-    DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, ToggleFocusMode, _toggleFocusModeHandlers, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::ToggleFocusModeEventArgs);
-    DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, ToggleFullscreen, _toggleFullscreenHandlers, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::ToggleFullscreenEventArgs);
-    DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, AlwaysOnTopChanged, _alwaysOnTopChangedHandlers, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::AlwaysOnTopChangedEventArgs);
+    DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, FocusModeChanged, _focusModeChangedHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
+    DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, FullscreenChanged, _fullscreenChangedHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
+    DEFINE_EVENT_WITH_TYPED_EVENT_HANDLER(TerminalPage, AlwaysOnTopChanged, _alwaysOnTopChangedHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
 }

src/cascadia/TerminalApp/TerminalPage.h

@@ -52,6 +52,8 @@ namespace winrt::TerminalApp::implementation
         void ToggleFocusMode();
         void ToggleFullscreen();
         void ToggleAlwaysOnTop();
+        bool FocusMode() const;
+        bool Fullscreen() const;
         bool AlwaysOnTop() const;
 
         void SetStartupActions(std::deque<winrt::TerminalApp::ActionAndArgs>& actions);
@@ -63,9 +65,9 @@ namespace winrt::TerminalApp::implementation
         DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(TitleChanged, _titleChangeHandlers, winrt::Windows::Foundation::IInspectable, winrt::hstring);
         DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(LastTabClosed, _lastTabClosedHandlers, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::LastTabClosedEventArgs);
         DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(SetTitleBarContent, _setTitleBarContentHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::UI::Xaml::UIElement);
-        DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(ToggleFocusMode, _toggleFocusModeHandlers, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::ToggleFocusModeEventArgs);
-        DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(ToggleFullscreen, _toggleFullscreenHandlers, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::ToggleFullscreenEventArgs);
-        DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(AlwaysOnTopChanged, _alwaysOnTopChangedHandlers, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::AlwaysOnTopChangedEventArgs);
+        DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(FocusModeChanged, _focusModeChangedHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
+        DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(FullscreenChanged, _fullscreenChangedHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
+        DECLARE_EVENT_WITH_TYPED_EVENT_HANDLER(AlwaysOnTopChanged, _alwaysOnTopChangedHandlers, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable);
         TYPED_EVENT(Initialized, winrt::Windows::Foundation::IInspectable, winrt::Windows::UI::Xaml::RoutedEventArgs);
 
     private:
@@ -90,7 +92,7 @@ namespace winrt::TerminalApp::implementation
 
         bool _isInFocusMode{ false };
         bool _isFullscreen{ false };
-        bool _isAlwayOnTop{ false };
+        bool _isAlwaysOnTop{ false };
 
         bool _rearranging;
         std::optional<int> _rearrangeFrom;

src/cascadia/TerminalApp/TerminalPage.idl

@@ -4,9 +4,6 @@
 namespace TerminalApp
 {
     delegate void LastTabClosedEventArgs();
-    delegate void ToggleFocusModeEventArgs();
-    delegate void ToggleFullscreenEventArgs();
-    delegate void AlwaysOnTopChangedEventArgs();
 
     interface IDialogPresenter
     {
@@ -21,6 +18,8 @@ namespace TerminalApp
         String ApplicationDisplayName { get; };
         String ApplicationVersion { get; };
 
+        Boolean FocusMode { get; };
+        Boolean Fullscreen { get; };
         Boolean AlwaysOnTop { get; };
 
         // We cannot use the default XAML APIs because we want to make sure
@@ -31,9 +30,9 @@ namespace TerminalApp
         event Windows.Foundation.TypedEventHandler<Object, String> TitleChanged;
         event Windows.Foundation.TypedEventHandler<Object, LastTabClosedEventArgs> LastTabClosed;
         event Windows.Foundation.TypedEventHandler<Object, Windows.UI.Xaml.UIElement> SetTitleBarContent;
-        event Windows.Foundation.TypedEventHandler<Object, ToggleFocusModeEventArgs> ToggleFocusMode;
-        event Windows.Foundation.TypedEventHandler<Object, ToggleFullscreenEventArgs> ToggleFullscreen;
-        event Windows.Foundation.TypedEventHandler<Object, AlwaysOnTopChangedEventArgs> AlwaysOnTopChanged;
+        event Windows.Foundation.TypedEventHandler<Object, Object> FocusModeChanged;
+        event Windows.Foundation.TypedEventHandler<Object, Object> FullscreenChanged;
+        event Windows.Foundation.TypedEventHandler<Object, Object> AlwaysOnTopChanged;
         event Windows.Foundation.TypedEventHandler<Object, Windows.UI.Xaml.RoutedEventArgs> Initialized;
     }
 }

src/cascadia/WindowsTerminal/AppHost.cpp

@@ -157,8 +157,8 @@ void AppHost::Initialize()
     _window->DragRegionClicked([this]() { _logic.TitlebarClicked(); });
 
     _logic.RequestedThemeChanged({ this, &AppHost::_UpdateTheme });
-    _logic.ToggleFullscreen({ this, &AppHost::_ToggleFullscreen });
-    _logic.ToggleFocusMode({ this, &AppHost::_ToggleFocusMode });
+    _logic.FullscreenChanged({ this, &AppHost::_FullscreenChanged });
+    _logic.FocusModeChanged({ this, &AppHost::_FocusModeChanged });
     _logic.AlwaysOnTopChanged({ this, &AppHost::_AlwaysOnTopChanged });
 
     _logic.Create();
@@ -355,20 +355,20 @@ void AppHost::_UpdateTheme(const winrt::Windows::Foundation::IInspectable&, cons
     _window->OnApplicationThemeChanged(arg);
 }
 
-void AppHost::_ToggleFocusMode(const winrt::Windows::Foundation::IInspectable&,
-                               const winrt::TerminalApp::ToggleFocusModeEventArgs&)
+void AppHost::_FocusModeChanged(const winrt::Windows::Foundation::IInspectable&,
+                                const winrt::Windows::Foundation::IInspectable&)
 {
-    _window->ToggleFocusMode();
+    _window->FocusModeChanged(_logic.FocusMode());
 }
 
-void AppHost::_ToggleFullscreen(const winrt::Windows::Foundation::IInspectable&,
-                                const winrt::TerminalApp::ToggleFullscreenEventArgs&)
+void AppHost::_FullscreenChanged(const winrt::Windows::Foundation::IInspectable&,
+                                 const winrt::Windows::Foundation::IInspectable&)
 {
-    _window->ToggleFullscreen();
+    _window->FullscreenChanged(_logic.Fullscreen());
 }
 
 void AppHost::_AlwaysOnTopChanged(const winrt::Windows::Foundation::IInspectable&,
-                                  const winrt::TerminalApp::AlwaysOnTopChangedEventArgs&)
+                                  const winrt::Windows::Foundation::IInspectable&)
 {
     _window->SetAlwaysOnTop(_logic.AlwaysOnTop());
 }

src/cascadia/WindowsTerminal/AppHost.h

@@ -33,11 +33,11 @@ class AppHost
                                 const winrt::Windows::UI::Xaml::UIElement& arg);
     void _UpdateTheme(const winrt::Windows::Foundation::IInspectable&,
                       const winrt::Windows::UI::Xaml::ElementTheme& arg);
-    void _ToggleFocusMode(const winrt::Windows::Foundation::IInspectable& sender,
-                          const winrt::TerminalApp::ToggleFocusModeEventArgs& arg);
-    void _ToggleFullscreen(const winrt::Windows::Foundation::IInspectable& sender,
-                           const winrt::TerminalApp::ToggleFullscreenEventArgs& arg);
+    void _FocusModeChanged(const winrt::Windows::Foundation::IInspectable& sender,
+                           const winrt::Windows::Foundation::IInspectable& arg);
+    void _FullscreenChanged(const winrt::Windows::Foundation::IInspectable& sender,
+                            const winrt::Windows::Foundation::IInspectable& arg);
     void _AlwaysOnTopChanged(const winrt::Windows::Foundation::IInspectable& sender,
-                             const winrt::TerminalApp::AlwaysOnTopChangedEventArgs& arg);
+                             const winrt::Windows::Foundation::IInspectable& arg);
     void _WindowMouseWheeled(const til::point coord, const int32_t delta);
 };

src/cascadia/WindowsTerminal/IslandWindow.cpp

@@ -443,25 +443,37 @@ void IslandWindow::OnApplicationThemeChanged(const winrt::Windows::UI::Xaml::Ele
 }
 
 // Method Description:
-// - Toggles our focus mode state. See _SetIsBorderless for more details.
+// - Updates our focus mode state. See _SetIsBorderless for more details.
 // Arguments:
 // - <none>
 // Return Value:
 // - <none>
-void IslandWindow::ToggleFocusMode()
+void IslandWindow::FocusModeChanged(const bool focusMode)
 {
-    _SetIsBorderless(!_borderless);
+    // Do nothing if the value was unchanged.
+    if (focusMode == _borderless)
+    {
+        return;
+    }
+
+    _SetIsBorderless(focusMode);
 }
 
 // Method Description:
-// - Toggles our fullscreen state. See _SetIsFullscreen for more details.
+// - Updates our fullscreen state. See _SetIsFullscreen for more details.
 // Arguments:
 // - <none>
 // Return Value:
 // - <none>
-void IslandWindow::ToggleFullscreen()
+void IslandWindow::FullscreenChanged(const bool fullscreen)
 {
-    _SetIsFullscreen(!_fullscreen);
+    // Do nothing if the value was unchanged.
+    if (fullscreen == _fullscreen)
+    {
+        return;
+    }
+
+    _SetIsFullscreen(fullscreen);
 }
 
 // Method Description:
@@ -480,13 +492,12 @@ void IslandWindow::SetAlwaysOnTop(const bool alwaysOnTop)
     const auto hwnd = GetHandle();
     if (hwnd)
     {
-        const til::rectangle windowPos{ GetWindowRect() };
         SetWindowPos(hwnd,
                      _alwaysOnTop ? HWND_TOPMOST : HWND_NOTOPMOST,
-                     windowPos.left<int>(),
-                     windowPos.top<int>(),
-                     windowPos.width<int>(),
-                     windowPos.height<int>(),
+                     0, // the window dimensions are unused, because we're passing SWP_NOSIZE
+                     0,
+                     0,
+                     0,
                      SWP_NOMOVE | SWP_NOSIZE);
     }
 }

src/cascadia/WindowsTerminal/IslandWindow.h

@@ -32,8 +32,8 @@ class IslandWindow :
     void SetCreateCallback(std::function<void(const HWND, const RECT, winrt::TerminalApp::LaunchMode& launchMode)> pfn) noexcept;
     void SetSnapDimensionCallback(std::function<float(bool widthOrHeight, float dimension)> pfn) noexcept;
 
-    void ToggleFocusMode();
-    void ToggleFullscreen();
+    void FocusModeChanged(const bool focusMode);
+    void FullscreenChanged(const bool fullscreen);
     void SetAlwaysOnTop(const bool alwaysOnTop);
 
 #pragma endregion