tons of cleanup

src/cascadia/TerminalApp/MinMaxCloseControl.cpp

@@ -138,17 +138,10 @@ namespace winrt::TerminalApp::implementation
             break;
         }
     }
-    void MinMaxCloseControl::ReleaseButton(CaptionButton button)
+    void MinMaxCloseControl::ReleaseButtons()
     {
-        switch (button)
+        VisualStateManager::GoToState(MinimizeButton(), L"Normal", false);
-        {
+        VisualStateManager::GoToState(MaximizeButton(), L"Normal", false);
-        case CaptionButton::Minimize:
+        VisualStateManager::GoToState(CloseButton(), L"Normal", false);
-        case CaptionButton::Maximize:
-        case CaptionButton::Close:
-            VisualStateManager::GoToState(MinimizeButton(), L"Normal", false);
-            VisualStateManager::GoToState(MaximizeButton(), L"Normal", false);
-            VisualStateManager::GoToState(CloseButton(), L"Normal", false);
-            break;
-        }
     }
 }

src/cascadia/TerminalApp/MinMaxCloseControl.h

@@ -22,7 +22,7 @@ namespace winrt::TerminalApp::implementation
 
         void HoverButton(CaptionButton button);
         void PressButton(CaptionButton button);
-        void ReleaseButton(CaptionButton button);
+        void ReleaseButtons();
 
         void _MinimizeClick(winrt::Windows::Foundation::IInspectable const& sender,
                             winrt::Windows::UI::Xaml::RoutedEventArgs const& e);

src/cascadia/TerminalApp/MinMaxCloseControl.idl

@@ -13,7 +13,7 @@ namespace TerminalApp
 
         void HoverButton(CaptionButton button);
         void PressButton(CaptionButton button);
-        void ReleaseButton(CaptionButton button);
+        void ReleaseButtons();
 
         event Windows.Foundation.TypedEventHandler<MinMaxCloseControl, Windows.UI.Xaml.RoutedEventArgs> MinimizeClick;
         event Windows.Foundation.TypedEventHandler<MinMaxCloseControl, Windows.UI.Xaml.RoutedEventArgs> MaximizeClick;

src/cascadia/TerminalApp/TitlebarControl.cpp

@@ -24,16 +24,6 @@ namespace winrt::TerminalApp::implementation
         MinMaxCloseControl().CloseClick({ this, &TitlebarControl::Close_Click });
     }
 
-    void TitlebarControl::MaxButtonEntered()
-    {
-        MinMaxCloseControl().Opacity(.5);
-    }
-
-    void TitlebarControl::MaxButtonExited()
-    {
-        MinMaxCloseControl().Opacity(1.0);
-    }
-
     IInspectable TitlebarControl::Content()
     {
         return ContentRoot().Content();
@@ -112,9 +102,24 @@ namespace winrt::TerminalApp::implementation
     {
         MinMaxCloseControl().PressButton(button);
     }
-    void TitlebarControl::ReleaseButton(CaptionButton button)
+    void TitlebarControl::ClickButton(CaptionButton button)
     {
-        MinMaxCloseControl().ReleaseButton(button);
+        switch (button)
+        {
+        case CaptionButton::Minimize:
+            Minimize_Click(nullptr, nullptr);
+            break;
+        case CaptionButton::Maximize:
+            Maximize_Click(nullptr, nullptr);
+            break;
+        case CaptionButton::Close:
+            Close_Click(nullptr, nullptr);
+            break;
+        }
+    }
+    void TitlebarControl::ReleaseButtons()
+    {
+        MinMaxCloseControl().ReleaseButtons();
     }
 
 }

src/cascadia/TerminalApp/TitlebarControl.h

@@ -14,11 +14,10 @@ namespace winrt::TerminalApp::implementation
     {
         TitlebarControl(uint64_t handle);
 
-        void MaxButtonEntered();
-        void MaxButtonExited();
         void HoverButton(CaptionButton button);
         void PressButton(CaptionButton button);
-        void ReleaseButton(CaptionButton button);
+        void ClickButton(CaptionButton button);
+        void ReleaseButtons();
 
         IInspectable Content();
         void Content(IInspectable content);

src/cascadia/TerminalApp/TitlebarControl.idl

@@ -11,9 +11,9 @@ namespace TerminalApp
     };
 
     enum CaptionButton {
-        Minimize,
+        Minimize = 8, // HTMINBUTTON
-        Maximize,
+        Maximize = 9, // HTMAXBUTTON
-        Close
+        Close = 20 // HTCLOSE
     };
 
     [default_interface] runtimeclass TitlebarControl : Windows.UI.Xaml.Controls.Grid
@@ -23,10 +23,8 @@ namespace TerminalApp
 
         void HoverButton(CaptionButton button);
         void PressButton(CaptionButton button);
-        void ReleaseButton(CaptionButton button);
+        void ClickButton(CaptionButton button);
-
+        void ReleaseButtons();
-        void MaxButtonEntered();
-        void MaxButtonExited();
 
         IInspectable Content;
         Windows.UI.Xaml.Controls.Border DragBar { get; };

src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp

@@ -122,90 +122,107 @@ LRESULT NonClientIslandWindow::_dragBarNcHitTest(const til::point& pointer)
 }
 
 // Function Description:
-// - The window procedure for the drag bar forwards clicks on its client area to its parent as non-client clicks.
+// - The window procedure for the drag bar forwards clicks on its client area to
+//   its parent as non-client clicks.
+// - BODGY: It also _manually_ handles the caption buttons. They exist in the
+//   titlebar, and work reasonably well with just XAML, if the drag bar isn't
+//   covering them.
+// - However, to get snap layout support (GH#9443), we need to actually return
+//   HTMAXBUTTON where the maximize button is. If the drag bar doesn't cover the
+//   caption buttons, then the core input site (which takes up the entirety of
+//   the XAML island) will steal the WM_NCHITTEST before we get a chance to
+//   handle it.
+// - So, the drag bar covers the caption buttons, and manually handles hovering
+//   and pressing them when needed. This gives the impression that they're
+//   getting input as they normally would, even if they're not _really_ getting
+//   input via XAML.
 LRESULT NonClientIslandWindow::_InputSinkMessageHandler(UINT const message, WPARAM const wparam, LPARAM const lparam) noexcept
 {
     switch (message)
     {
     case WM_NCHITTEST:
     {
-        const til::point pointer{ GET_X_LPARAM(lparam), GET_Y_LPARAM(lparam) };
+        // Try to determine what part of the window is being hovered here. This
-
+        // is absolutely critical to making sure Snap Layouts (GH#9443) works!
-        return _dragBarNcHitTest(pointer);
+        return _dragBarNcHitTest(til::point{ GET_X_LPARAM(lparam), GET_Y_LPARAM(lparam) });
     }
     break;
 
     case WM_NCMOUSEMOVE:
-        // Communicate state to the title bar control so that it can update its visuals.
+        // When we get this message, it's because the mouse moved when it was
-        // TODO! other buttons too
+        // over somewhere we said was the non-client area.
-
+        //
+        // We'll use this to communicate state to the title bar control, so that
+        // it can update its visuals.
+        // - If we're over a button, hover it.
+        // - If we're over _anything else_, stop hovering the buttons.
         switch (wparam)
         {
         case HTMINBUTTON:
             _titlebar.HoverButton(winrt::TerminalApp::CaptionButton::Minimize);
-            // _titlebar.ReleaseButton(winrt::TerminalApp::CaptionButton::Maximize);
-            // _titlebar.ReleaseButton(winrt::TerminalApp::CaptionButton::Close);
             break;
         case HTMAXBUTTON:
-            // _titlebar.ReleaseButton(winrt::TerminalApp::CaptionButton::Minimize);
             _titlebar.HoverButton(winrt::TerminalApp::CaptionButton::Maximize);
-            // _titlebar.ReleaseButton(winrt::TerminalApp::CaptionButton::Close);
             break;
         case HTCLOSE:
-            // _titlebar.ReleaseButton(winrt::TerminalApp::CaptionButton::Minimize);
-            // _titlebar.ReleaseButton(winrt::TerminalApp::CaptionButton::Maximize);
             _titlebar.HoverButton(winrt::TerminalApp::CaptionButton::Close);
             break;
         default:
-            _titlebar.ReleaseButton(winrt::TerminalApp::CaptionButton::Close);
+            _titlebar.ReleaseButtons();
         }
 
-        if (!_trackingMouse && (wparam == HTMINBUTTON ||
+        // If we haven't previously asked for mouse tracking, request mouse
-            wparam == HTMAXBUTTON ||
+        // tracking. We need to do this so we can get the WM_NCMOUSELEAVE
-            wparam == HTCLOSE))
+        // message when the mouse leave the titlebar. Otherwise, we won't always
+        // get that message (especially if the user moves the mouse _real
+        // fast_).
+        if (!_trackingMouse &&
+            (wparam == HTMINBUTTON || wparam == HTMAXBUTTON || wparam == HTCLOSE))
         {
-            _trackingMouse = true;
             TRACKMOUSEEVENT ev{};
             ev.cbSize = sizeof(TRACKMOUSEEVENT);
+            // TME_NONCLIENT is absolutely critical here. In my experimentation,
+            // we'd get WM_MOUSELEAVE messages after just a HOVER_DEFAULT
+            // timeout even though we're not requesting TME_HOVER, which kinda
+            // ruined the whole point of this.
             ev.dwFlags = TME_LEAVE | TME_NONCLIENT;
             ev.hwndTrack = _dragBarWindow.get();
             ev.dwHoverTime = HOVER_DEFAULT; // we don't _really_ care about this.
             LOG_IF_WIN32_BOOL_FALSE(TrackMouseEvent(&ev));
+            _trackingMouse = true;
         }
-
+        // TODO! We no longer show tooltips for the caption buttons when they're
-        // if (wparam == HTMAXBUTTON)
+        // hovered. That's not good.
-        // {
-        //     _titlebar.HoverButton(winrt::TerminalApp::CaptionButton::Maximize);
-        //     // _titlebar.MaxButtonEntered();
-        // }
-        // else
-        // {
-        //     _titlebar.ReleaseButton(winrt::TerminalApp::CaptionButton::Maximize);
-        //     // _titlebar.MaxButtonExited();
-        // }
         break;
 
     case WM_NCMOUSELEAVE:
     case WM_MOUSELEAVE:
-        // _titlebar.MaxButtonExited();
+        // When the mouse leaves the drag rect, make sure to dismiss any hover.
-        // _titlebar.ReleaseButton(winrt::TerminalApp::CaptionButton::Minimize);
+        _titlebar.ReleaseButtons();
-        // _titlebar.ReleaseButton(winrt::TerminalApp::CaptionButton::Maximize);
-        _titlebar.ReleaseButton(winrt::TerminalApp::CaptionButton::Close);
         _trackingMouse = false;
         break;
 
-    // NB: *Shouldn't be forwarding these* when they're not over the caption because they can inadvertently take action using the system's default metrics instead of our own.
+    // NB: *Shouldn't be forwarding these* when they're not over the caption
+    // because they can inadvertently take action using the system's default
+    // metrics instead of our own.
     case WM_NCLBUTTONDOWN:
     case WM_NCLBUTTONDBLCLK:
+        // Manual handling for mouse clicks in the drag bar. If it's in a
+        // caption button, then tell the titlebar to "press" the button, which
+        // should change its visual state.
+        //
+        // If it's not in a caption button, then just forward the message along
+        // to the root HWND.
         switch (wparam)
         {
         case HTCAPTION:
         {
             // Pass caption-related nonclient messages to the parent window.
-            // The buttons won't work as you'd expect; we need to handle those ourselves.
             auto parentWindow{ GetHandle() };
             return SendMessage(parentWindow, message, wparam, lparam);
         }
+        // The buttons won't work as you'd expect; we need to handle those
+        // ourselves.
         case HTMINBUTTON:
             _titlebar.PressButton(winrt::TerminalApp::CaptionButton::Minimize);
             break;
@@ -217,8 +234,13 @@ LRESULT NonClientIslandWindow::_InputSinkMessageHandler(UINT const message, WPAR
             break;
         }
         return 0;
-    // TODO!: I think we only want WM_NCLBUTTONUP
+
     case WM_NCLBUTTONUP:
+        // Manual handling for mouse RELEASES in the drag bar. If it's in a
+        // caption button, then manually handle what we'd expect for that button.
+        //
+        // If it's not in a caption button, then just forward the message along
+        // to the root HWND.
         switch (wparam)
         {
         case HTCAPTION:
@@ -233,20 +255,23 @@ LRESULT NonClientIslandWindow::_InputSinkMessageHandler(UINT const message, WPAR
         // Forward along to the button state machine.
         // As a proof of concept just locally handle the maximize button.
         case HTMINBUTTON:
-            _titlebar.ReleaseButton(winrt::TerminalApp::CaptionButton::Minimize);
+            _titlebar.ClickButton(winrt::TerminalApp::CaptionButton::Minimize);
-            ShowWindow(GetHandle(), SW_MINIMIZE);
+            _titlebar.ReleaseButtons();
+            // ShowWindow(GetHandle(), SW_MINIMIZE);
             break;
 
         case HTMAXBUTTON:
-            _titlebar.ReleaseButton(winrt::TerminalApp::CaptionButton::Maximize);
+            _titlebar.ClickButton(winrt::TerminalApp::CaptionButton::Maximize);
+            _titlebar.ReleaseButtons();
             // If we're maximized, restore down. Otherwise, maximize
-            ShowWindow(GetHandle(), IsZoomed(GetHandle()) ? SW_RESTORE : SW_MAXIMIZE);
+            // ShowWindow(GetHandle(), IsZoomed(GetHandle()) ? SW_RESTORE : SW_MAXIMIZE);
             break;
 
         case HTCLOSE:
             // TODO! this seems to crash unreasonably frequently
-            _titlebar.ReleaseButton(winrt::TerminalApp::CaptionButton::Close);
+            _titlebar.ClickButton(winrt::TerminalApp::CaptionButton::Close);
-            Close();
+            _titlebar.ReleaseButtons();
+            // Close();
             break;
         }
         return 0;