cleanup for review

src/cascadia/TerminalApp/Pane.cpp

@@ -843,7 +843,7 @@ void Pane::_UpdateBorders()
     Thickness newBorders{ 0 };
     if (_zoomed)
     {
-        // TODO: Make this 0, and use the magnifying glass as the zoomed icon
+        // When the pane is zoomed, manually show all the borders around the window.
         top = bottom = right = left = PaneBorderSize;
     }
     else
@@ -1181,56 +1181,74 @@ std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> Pane::_Split(SplitState
     return { _firstChild, _secondChild };
 }
 
+// Method Description:
+// - Recursively attempt to "zoom" the given pane. When the pane is zoomed, it
+//   won't be displayed as part of the tab tree, instead it'll take up the full
+//   content of the tab. When we find the given pane, we'll need to remove it
+//   from the UI tree, so that the caller can re-add it. We'll also set some
+//   internal state, so the pane can display all of its borders.
+// Arguments:
+// - zoomedPane: This is the pane which we're attempting to zoom on.
+// Return Value:
+// - <none>
 void Pane::Zoom(std::shared_ptr<Pane> zoomedPane)
 {
-    // Do nothing if we're the only pane in the tree.
-
-    // TODO: Can we just do nothing if we're a leaf? is that the equivalent of the above?
     if (_IsLeaf())
     {
         _zoomed = (zoomedPane == shared_from_this());
         _UpdateBorders();
-        if (_zoomed)
+    }
+    else
+    {
+        if (zoomedPane == _firstChild || zoomedPane == _secondChild)
         {
+            // When we're zooming the pane, we'll need to remove it from our UI
+            // tree. Easy way: just remove both children. We'll re-attach both
+            // when we unzoom.
+            _root.Children().Clear();
         }
-        return;
-    }
 
-    if (zoomedPane == _firstChild || zoomedPane == _secondChild)
-    {
-        // When we're unzooming the pane, we'll need to re-add it to our UI tree where it originally belonged.
-        // easy way: just re-add both:
-        _root.Children().Clear();
+        // Always recurse into both children. If the (un)zoomed pane was one of
+        // our direct children, we'll still want to update it's borders.
+        _firstChild->Zoom(zoomedPane);
+        _secondChild->Zoom(zoomedPane);
     }
-    // else
-    // {
-    _firstChild->Zoom(zoomedPane);
-    _secondChild->Zoom(zoomedPane);
-    // }
 }
 
+// Method Description:
+// - Recursively attempt to "unzoom" the given pane. This does the opposite of
+//   Pane::Zoom. When we find the given pane, we should return the pane to our
+//   UI tree. We'll also clear the internal state, so the pane can display its
+//   borders correctly.
+// - The caller should make sure to have removed the zoomed pane from the UI
+//   tree _before_ calling this.
+// Arguments:
+// - zoomedPane: This is the pane which we're attempting to unzoom.
+// Return Value:
+// - <none>
 void Pane::UnZoom(std::shared_ptr<Pane> zoomedPane)
 {
     if (_IsLeaf())
     {
         _zoomed = false;
         _UpdateBorders();
-        return;
     }
-
-    if (zoomedPane == _firstChild || zoomedPane == _secondChild)
+    else
     {
-        // When we're unzooming the pane, we'll need to re-add it to our UI tree where it originally belonged.
-        // easy way: just re-add both:
-        _root.Children().Clear();
-        _root.Children().Append(_firstChild->GetRootElement());
-        _root.Children().Append(_secondChild->GetRootElement());
+        if (zoomedPane == _firstChild || zoomedPane == _secondChild)
+        {
+            // When we're unzoom-ing the pane, we'll need to re-add it to our UI
+            // tree where it originally belonged. easy way: just re-add both.
+            _root.Children().Clear();
+            _root.Children().Append(_firstChild->GetRootElement());
+            _root.Children().Append(_secondChild->GetRootElement());
+        }
+
+        // Always recurse into both children. If the (un)zoomed pane was one of
+        // our direct children, we'll still want to update it's borders.
+        _firstChild->UnZoom(zoomedPane);
+        _secondChild->UnZoom(zoomedPane);
     }
-    // else
-    // {
-    _firstChild->UnZoom(zoomedPane);
-    _secondChild->UnZoom(zoomedPane);
-    // }
 }
 
 // Method Description:

src/cascadia/TerminalApp/Tab.cpp

@@ -906,6 +906,16 @@ namespace winrt::TerminalApp::implementation
         return _rootPane->PreCalculateCanSplit(_activePane, splitType, availableSpace).value_or(false);
     }
 
+    // Method Description:
+    // - Toggle our zoom state.
+    //   * If we're not zoomed, then zoom the active pane, making it take the
+    //     full size of the tab. We'll achieve this by changing our response to
+    //     Tab::GetRootElement, so that it'll return the zoomed pane only.
+    //   *  If we're currently zoomed on a pane, unzoom that pane.
+    // Arguments:
+    // - <none>
+    // Return Value:
+    // - <none>
     void Tab::ToggleZoom()
     {
         if (_zoomedPane)
@@ -917,17 +927,18 @@ namespace winrt::TerminalApp::implementation
             EnterZoom();
         }
     }
-
     void Tab::EnterZoom()
     {
         _zoomedPane = _activePane;
         _rootPane->Zoom(_zoomedPane);
+        // Update the tab header to show the magnifying glass
         _UpdateTabHeader();
     }
     void Tab::ExitZoom()
     {
         _rootPane->UnZoom(_zoomedPane);
         _zoomedPane = nullptr;
+        // Update the tab header to hide the magnifying glass
         _UpdateTabHeader();
     }
 

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -1168,13 +1168,29 @@ namespace winrt::TerminalApp::implementation
         return false;
     }
 
+    // Method Description:
+    // - Helper to manually exit "zoom" when certain actions take place.
+    //   Anything that modifies the state of the pane tree should probably
+    //   unzoom the focused pane first, so that the user can see the full pane
+    //   tree again. These actions include:
+    //   * Splitting a new pane
+    //   * Closing a pane
+    //   * Moving focus between panes
+    //   * Resizing a pane
+    // Arguments:
+    // - <none>
+    // Return Value:
+    // - <none>
     void TerminalPage::_UnZoomIfNeeded()
     {
         auto activeTab = _GetFocusedTab();
         if (activeTab && activeTab->IsZoomed())
         {
+            // Remove the content from the tab first, so Pane::UnZoom can
+            // re-attach the content to the tree w/in the pane
             _tabContent.Children().Clear();
             activeTab->ExitZoom();
+            // Re-attach the tab's content to the UI tree.
             _tabContent.Children().Append(activeTab->GetRootElement());
         }
     }