From 768854af32d27a4b7428aeb2f1aa7e87598d7802 Mon Sep 17 00:00:00 2001
From: Marcel Wagner <marcel.alex.wagner@outlook.com>
Date: Sat, 21 Mar 2020 16:45:45 +0100
Subject: [PATCH] Numberbox header aligning (#2117)

* Fix issue with header taking up space when not specified/null

* Add NumberBox header test

* Add log comments for test

* Fix behavior of empty string

* Update test
---
 dev/Generated/NumberBox.properties.cpp        | 10 ++++-
 dev/Generated/NumberBox.properties.h          |  4 ++
 .../InteractionTests/NumberBoxTests.cs        | 30 ++++++++++++++
 dev/NumberBox/NumberBox.cpp                   | 41 +++++++++++++++++++
 dev/NumberBox/NumberBox.h                     |  2 +
 dev/NumberBox/NumberBox.idl                   |  1 +
 dev/NumberBox/NumberBox.xaml                  |  4 +-
 dev/NumberBox/TestUI/NumberBoxPage.xaml       |  8 ++++
 dev/NumberBox/TestUI/NumberBoxPage.xaml.cs    | 24 +++++++++++
 9 files changed, 122 insertions(+), 2 deletions(-)

diff --git a/dev/Generated/NumberBox.properties.cpp b/dev/Generated/NumberBox.properties.cpp
index 01df4ba7c8..323b3d557a 100644
--- a/dev/Generated/NumberBox.properties.cpp
+++ b/dev/Generated/NumberBox.properties.cpp
@@ -72,7 +72,7 @@ void NumberBoxProperties::EnsureProperties()
                 winrt::name_of<winrt::NumberBox>(),
                 false /* isAttached */,
                 ValueHelper<winrt::IInspectable>::BoxedDefaultValue(),
-                nullptr);
+                winrt::PropertyChangedCallback(&OnHeaderPropertyChanged));
     }
     if (!s_HeaderTemplateProperty)
     {
@@ -275,6 +275,14 @@ void NumberBoxProperties::ClearProperties()
     s_ValueProperty = nullptr;
 }
 
+void NumberBoxProperties::OnHeaderPropertyChanged(
+    winrt::DependencyObject const& sender,
+    winrt::DependencyPropertyChangedEventArgs const& args)
+{
+    auto owner = sender.as<winrt::NumberBox>();
+    winrt::get_self<NumberBox>(owner)->OnHeaderPropertyChanged(args);
+}
+
 void NumberBoxProperties::OnIsWrapEnabledPropertyChanged(
     winrt::DependencyObject const& sender,
     winrt::DependencyPropertyChangedEventArgs const& args)
diff --git a/dev/Generated/NumberBox.properties.h b/dev/Generated/NumberBox.properties.h
index fb9c4757db..ed33776413 100644
--- a/dev/Generated/NumberBox.properties.h
+++ b/dev/Generated/NumberBox.properties.h
@@ -114,6 +114,10 @@ class NumberBoxProperties
     static void EnsureProperties();
     static void ClearProperties();
 
+    static void OnHeaderPropertyChanged(
+        winrt::DependencyObject const& sender,
+        winrt::DependencyPropertyChangedEventArgs const& args);
+
     static void OnIsWrapEnabledPropertyChanged(
         winrt::DependencyObject const& sender,
         winrt::DependencyPropertyChangedEventArgs const& args);
diff --git a/dev/NumberBox/InteractionTests/NumberBoxTests.cs b/dev/NumberBox/InteractionTests/NumberBoxTests.cs
index c0e0e8baf4..2941c6c497 100644
--- a/dev/NumberBox/InteractionTests/NumberBoxTests.cs
+++ b/dev/NumberBox/InteractionTests/NumberBoxTests.cs
@@ -533,6 +533,36 @@ public void BasicExpressionTest()
             }
         }
 
+        [TestMethod]
+        public void VerifyNumberBoxHeaderBehavior()
+        {
+            using (var setup = new TestSetupHelper("NumberBox Tests"))
+            {
+                var toggleHeaderButton = FindElement.ByName<Button>("ToggleHeaderValueButton");
+                var header = FindElement.ByName<TextBlock>("NumberBoxHeaderClippingDemoHeader");
+
+                Log.Comment("Check header is null");
+                Verify.IsNull(header);
+
+                Log.Comment("Set header");
+                toggleHeaderButton.Invoke();
+                Wait.ForIdle();
+                
+                header = FindElement.ByName<TextBlock>("NumberBoxHeaderClippingDemoHeader");
+                Log.Comment("Check if header is present");
+                Verify.IsNotNull(header);
+                Log.Comment("Remove header");
+                toggleHeaderButton.Invoke();
+                Wait.ForIdle();
+                ElementCache.Clear();
+
+                Log.Comment("Check that header is null again");
+                header = FindElement.ByName<TextBlock>("NumberBoxHeaderClippingDemoHeader");
+                Verify.IsNull(header);
+            }
+        }
+
+
         Button FindButton(UIObject parent, string buttonName)
         {
             foreach (UIObject elem in parent.Children)
diff --git a/dev/NumberBox/NumberBox.cpp b/dev/NumberBox/NumberBox.cpp
index bc226ec627..3ad7b4c394 100644
--- a/dev/NumberBox/NumberBox.cpp
+++ b/dev/NumberBox/NumberBox.cpp
@@ -11,6 +11,7 @@
 #include "Utils.h"
 #include "winnls.h"
 
+static constexpr wstring_view c_numberBoxHeaderName{ L"HeaderContentPresenter"sv };
 static constexpr wstring_view c_numberBoxDownButtonName{ L"DownSpinButton"sv };
 static constexpr wstring_view c_numberBoxUpButtonName{ L"UpSpinButton"sv };
 static constexpr wstring_view c_numberBoxTextBoxName{ L"InputBox"sv };
@@ -135,6 +136,12 @@ void NumberBox::OnApplyTemplate()
         }
     }
 
+    if (const auto headerPresenter = GetTemplateChildT<winrt::ContentPresenter>(c_numberBoxHeaderName, controlProtected))
+    {
+        // Set presenter to enable lightweight styling of the headers margin
+        m_headerPresenter.set(headerPresenter);
+    }
+
     m_textBox.set([this, controlProtected]() {
         const auto textBox = GetTemplateChildT<winrt::TextBox>(c_numberBoxTextBoxName, controlProtected);
         if (textBox)
@@ -298,6 +305,40 @@ void NumberBox::UpdateValueToText()
     }
 }
 
+void NumberBox::OnHeaderPropertyChanged(const winrt::DependencyPropertyChangedEventArgs& args)
+{
+    // To enable lightweight styling, collapse header presenter if there is no header specified
+    if (const auto headerPresenter = m_headerPresenter.get())
+    {
+        if (const auto header = Header())
+        {
+            // Check if header is string or not
+            if (const auto headerAsString = Header().try_as<winrt::IReference<winrt::hstring>>())
+            {
+                if (headerAsString.Value().empty())
+                {
+                    // String is the empty string, hide presenter
+                    headerPresenter.Visibility(winrt::Visibility::Collapsed);
+                }
+                else
+                {
+                    // String is not an empty string
+                    headerPresenter.Visibility(winrt::Visibility::Visible);
+                }
+            }
+            else
+            {
+                // Header is not a string, so let's show header presenter
+                headerPresenter.Visibility(winrt::Visibility::Visible);
+            }
+        }
+        else
+        {
+            headerPresenter.Visibility(winrt::Visibility::Collapsed);
+        }
+    }
+}
+
 void NumberBox::OnValidationModePropertyChanged(const winrt::DependencyPropertyChangedEventArgs& args)
 {
     ValidateInput();
diff --git a/dev/NumberBox/NumberBox.h b/dev/NumberBox/NumberBox.h
index 85a034962a..5128c40e9c 100644
--- a/dev/NumberBox/NumberBox.h
+++ b/dev/NumberBox/NumberBox.h
@@ -42,6 +42,7 @@ class NumberBox :
     // IFrameworkElement
     void OnApplyTemplate();
 
+    void OnHeaderPropertyChanged(const winrt::DependencyPropertyChangedEventArgs& args);
     void OnSpinButtonPlacementModePropertyChanged(const winrt::DependencyPropertyChangedEventArgs& args);
     void OnTextPropertyChanged(const winrt::DependencyPropertyChangedEventArgs& args);
 
@@ -87,6 +88,7 @@ class NumberBox :
     winrt::SignificantDigitsNumberRounder m_displayRounder{};
 
     tracker_ref<winrt::TextBox> m_textBox{ this };
+    tracker_ref<winrt::ContentPresenter> m_headerPresenter{ this };
     tracker_ref<winrt::Popup> m_popup{ this };
 
     winrt::RepeatButton::Click_revoker m_upButtonClickRevoker{};
diff --git a/dev/NumberBox/NumberBox.idl b/dev/NumberBox/NumberBox.idl
index 47903c3a81..90138e4920 100644
--- a/dev/NumberBox/NumberBox.idl
+++ b/dev/NumberBox/NumberBox.idl
@@ -91,6 +91,7 @@ unsealed runtimeclass NumberBox : Windows.UI.Xaml.Controls.Control
     static Windows.UI.Xaml.DependencyProperty LargeChangeProperty{ get; };
     static Windows.UI.Xaml.DependencyProperty TextProperty{ get; };
 
+    [MUX_PROPERTY_CHANGED_CALLBACK(TRUE)]
     static Windows.UI.Xaml.DependencyProperty HeaderProperty{ get; };
     static Windows.UI.Xaml.DependencyProperty HeaderTemplateProperty{ get; };
     static Windows.UI.Xaml.DependencyProperty PlaceholderTextProperty{ get; };
diff --git a/dev/NumberBox/NumberBox.xaml b/dev/NumberBox/NumberBox.xaml
index b3a7944cf1..801f6820a3 100644
--- a/dev/NumberBox/NumberBox.xaml
+++ b/dev/NumberBox/NumberBox.xaml
@@ -99,7 +99,9 @@
                             Foreground="{ThemeResource TextControlHeaderForeground}"
                             Margin="{ThemeResource TextBoxTopHeaderMargin}"
                             TextWrapping="Wrap"
-                            VerticalAlignment="Top"/>
+                            VerticalAlignment="Top"
+                            Visibility="Collapsed"
+                            x:DeferLoadStrategy="Lazy" />
 
                         <TextBox x:Name="InputBox"
                             Grid.Row="1"
diff --git a/dev/NumberBox/TestUI/NumberBoxPage.xaml b/dev/NumberBox/TestUI/NumberBoxPage.xaml
index be42bb7e81..ef01d35984 100644
--- a/dev/NumberBox/TestUI/NumberBoxPage.xaml
+++ b/dev/NumberBox/TestUI/NumberBoxPage.xaml
@@ -61,6 +61,8 @@
             <Button x:Name="SetValueNaNButton" AutomationProperties.Name="SetValueNaNButton" Content="Set value to NaN" Click="SetNaNButton_Click" Margin="0,4,0,0"/>
 
             <Button x:Name="SetTwoWayBoundValueNaNButton" AutomationProperties.Name="SetTwoWayBoundValueNaNButton" Content="Set two way bound value to NaN" Click="SetTwoWayBoundNaNButton_Click" Margin="0,4,0,0"/>
+
+            <Button x:Name="ToggleHeaderValueButton" AutomationProperties.Name="ToggleHeaderValueButton" Content="Toggle header for clipping issue" Click="ToggleHeaderValueButton_Click" Margin="0,4,0,0"/>
         </StackPanel>
 
         <Grid Grid.Column="1">
@@ -122,6 +124,12 @@
                               Value="{x:Bind DataModelWithINPC.Value, Mode=TwoWay}" />
                     <TextBlock x:Name="TwoWayBoundNumberBoxValue" AutomationProperties.Name="TwoWayBoundNumberBoxValue" />
                 </StackPanel>
+
+                <!-- Testing alignment with textbox and without specified header -->
+                <StackPanel Orientation="Horizontal">
+                    <TextBox MaxHeight="30" VerticalAlignment="Top"/>
+                    <controls:NumberBox x:Name="HeaderTestingNumberBox" MaxHeight="32" VerticalAlignment="Top" PlaceholderText="I should not be clipped without header"/>
+                </StackPanel>
             </StackPanel>
         </Grid>
 
diff --git a/dev/NumberBox/TestUI/NumberBoxPage.xaml.cs b/dev/NumberBox/TestUI/NumberBoxPage.xaml.cs
index 92986a2288..c37a16a1c0 100644
--- a/dev/NumberBox/TestUI/NumberBoxPage.xaml.cs
+++ b/dev/NumberBox/TestUI/NumberBoxPage.xaml.cs
@@ -7,6 +7,7 @@
 using System.ComponentModel;
 using Windows.Globalization.NumberFormatting;
 using Windows.UI.Xaml;
+using Windows.UI.Xaml.Automation;
 using Windows.UI.Xaml.Controls;
 
 namespace MUXControlsTestApp
@@ -124,6 +125,29 @@ private void SetTwoWayBoundNaNButton_Click(object sender, RoutedEventArgs e)
             TwoWayBoundNumberBoxValue.Text = TwoWayBoundNumberBox.Value.ToString();
         }
 
+        private void ToggleHeaderValueButton_Click(object sender, RoutedEventArgs e)
+        {
+            if(HeaderTestingNumberBox.Header is null)
+            {
+                var demoHeader = new TextBlock();
+                demoHeader.SetValue(AutomationProperties.NameProperty, "NumberBoxHeaderClippingDemoHeader");
+                demoHeader.Text = "Test header";
+                HeaderTestingNumberBox.Header = demoHeader;
+            }
+            else
+            {
+                // Switching between normal header and empty string header
+                if(HeaderTestingNumberBox.Header as string is null)
+                {
+                    HeaderTestingNumberBox.Header = "";
+                }
+                else
+                {
+                    HeaderTestingNumberBox.Header = null;
+                }
+            }
+        }
+
         private void TextPropertyChanged(DependencyObject o, DependencyProperty p)
         {
             TextTextBox.Text = TestNumberBox.Text;