From 2a0637e51c13a157336a5fd2e3aa28c040b953f6 Mon Sep 17 00:00:00 2001
From: Pepe Rivera <joriv@microsoft.com>
Date: Thu, 12 Dec 2019 12:58:28 -0800
Subject: [PATCH] Fix some UI bugs in graph mode (#862)

* Address issues with error states

* Add various fixes

* Add back initial tooltip

* PR comments
---
 src/Calculator/App.xaml                       | 12 +++++++---
 src/Calculator/Controls/EquationTextBox.cpp   | 13 +++++++++--
 src/Calculator/Resources/en-US/Resources.resw |  6 ++---
 .../GraphingCalculator.xaml                   |  1 +
 .../GraphingCalculator.xaml.cpp               |  9 +++++++-
 .../KeyGraphFeaturesPanel.xaml                | 22 ++++++++++++-------
 src/Calculator/pch.h                          |  1 +
 src/GraphControl/Control/Grapher.cpp          |  9 ++------
 src/GraphControl/DirectX/RenderMain.cpp       | 10 ++++++++-
 9 files changed, 58 insertions(+), 25 deletions(-)

diff --git a/src/Calculator/App.xaml b/src/Calculator/App.xaml
index 957adc6..0398438 100644
--- a/src/Calculator/App.xaml
+++ b/src/Calculator/App.xaml
@@ -1923,9 +1923,9 @@
                                         </VisualState>
                                         <VisualState x:Name="PointerOverError">
                                             <VisualState.Setters>
-                                                <Setter Target="EquationBoxBorder.BorderBrush" Value="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=EquationColor}"/>
+                                                <Setter Target="EquationBoxBorder.BorderBrush" Value="{ThemeResource EquationBoxErrorBorderBrush}"/>
                                                 <Setter Target="EquationBoxBorder.Background" Value="{ThemeResource EquationBoxErrorBackgroundBrush}"/>
-                                                <Setter Target="ColorChooserButton.Visibility" Value="Visible"/>
+                                                <Setter Target="ColorChooserButton.Visibility" Value="Collapsed"/>
                                                 <Setter Target="FunctionButton.Visibility" Value="Collapsed"/>
                                                 <Setter Target="RemoveButton.Visibility" Value="Visible"/>
                                                 <Setter Target="ErrorIcon.Visibility" Value="Collapsed"/>
@@ -1944,12 +1944,18 @@
                                                 <Setter Target="EquationBoxBorder.Background" Value="{ThemeResource TextBoxBackgroundThemeBrush}"/>
                                             </VisualState.Setters>
                                         </VisualState>
+                                        <VisualState x:Name="FocusedError">
+                                            <VisualState.Setters>
+                                                <Setter Target="EquationBoxBorder.BorderBrush" Value="{ThemeResource EquationBoxErrorBorderBrush}"/>
+                                                <Setter Target="EquationBoxBorder.Background" Value="{ThemeResource TextBoxBackgroundThemeBrush}"/>
+                                                <Setter Target="ErrorIcon.Visibility" Value="Collapsed"/>
+                                            </VisualState.Setters>
+                                        </VisualState>
                                     </VisualStateGroup>
                                     <VisualStateGroup x:Name="ButtonStates">
                                         <VisualState x:Name="ButtonVisible">
                                             <VisualState.Setters>
                                                 <Setter Target="DeleteButton.Visibility" Value="Visible"/>
-                                                <Setter Target="ErrorIcon.Visibility" Value="Collapsed"/>
                                             </VisualState.Setters>
                                         </VisualState>
                                         <VisualState x:Name="ButtonHideRemove">
diff --git a/src/Calculator/Controls/EquationTextBox.cpp b/src/Calculator/Controls/EquationTextBox.cpp
index 2ab23b9..2663c76 100644
--- a/src/Calculator/Controls/EquationTextBox.cpp
+++ b/src/Calculator/Controls/EquationTextBox.cpp
@@ -315,7 +315,11 @@ void EquationTextBox::UpdateCommonVisualState()
 {
     String ^ state = nullptr;
 
-    if (m_HasFocus)
+    if (m_HasFocus && HasError)
+    {
+        state = "FocusedError";
+    }
+    else if (m_HasFocus)
     {
         state = "Focused";
     }
@@ -390,7 +394,12 @@ void EquationTextBox::OnRichEditMenuOpening(Object ^ /*sender*/, Object ^ /*args
 {
     if (m_kgfEquationMenuItem != nullptr)
     {
-        m_kgfEquationMenuItem->IsEnabled = EquationTextBox::RichEditHasContent();
+        m_kgfEquationMenuItem->IsEnabled = RichEditHasContent();
+    }
+
+    if (m_colorChooserMenuItem != nullptr)
+    {
+        m_colorChooserMenuItem->IsEnabled = !HasError;
     }
 }
 
diff --git a/src/Calculator/Resources/en-US/Resources.resw b/src/Calculator/Resources/en-US/Resources.resw
index 458d763..8e6414d 100644
--- a/src/Calculator/Resources/en-US/Resources.resw
+++ b/src/Calculator/Resources/en-US/Resources.resw
@@ -4096,11 +4096,11 @@
     <comment>Title for KeyGraphFeatures Vertical Asymptotes Property</comment>
   </data>
   <data name="XIntercept" xml:space="preserve">
-    <value>X Intercept</value>
+    <value>X-Intercept</value>
     <comment>Title for KeyGraphFeatures XIntercept Property</comment>
   </data>
   <data name="YIntercept" xml:space="preserve">
-    <value>Y Intercept</value>
+    <value>Y-Intercept</value>
     <comment>Title for KeyGraphFeatures YIntercept Property</comment>
   </data>
   <data name="KGFAnalysisCouldNotBePerformed" xml:space="preserve">
@@ -4222,4 +4222,4 @@
     <value>Enter an equation</value>
     <comment>Used in the Graphing Calculator to indicate to users that they can enter an equation in the textbox</comment>
   </data>
-</root>
\ No newline at end of file
+</root>
diff --git a/src/Calculator/Views/GraphingCalculator/GraphingCalculator.xaml b/src/Calculator/Views/GraphingCalculator/GraphingCalculator.xaml
index aeff790..e41b8c6 100644
--- a/src/Calculator/Views/GraphingCalculator/GraphingCalculator.xaml
+++ b/src/Calculator/Views/GraphingCalculator/GraphingCalculator.xaml
@@ -650,6 +650,7 @@
             <!-- Ideally the KeyGraphFeaturesPanel should be a frame so that navigation to and from the panel could be handled nicely -->
             <local:KeyGraphFeaturesPanel x:Name="KeyGraphFeaturesControl"
                                          Grid.RowSpan="2"
+                                         Margin="0,4,0,0"
                                          KeyGraphFeaturesClosed="OnKeyGraphFeaturesClosed"
                                          ViewModel="{x:Bind EquationInputAreaControl.EquationVM, Mode=OneWay}"
                                          Visibility="{x:Bind IsKeyGraphFeaturesVisible, Mode=OneWay}"
diff --git a/src/Calculator/Views/GraphingCalculator/GraphingCalculator.xaml.cpp b/src/Calculator/Views/GraphingCalculator/GraphingCalculator.xaml.cpp
index 761ee8e..f07c665 100644
--- a/src/Calculator/Views/GraphingCalculator/GraphingCalculator.xaml.cpp
+++ b/src/Calculator/Views/GraphingCalculator/GraphingCalculator.xaml.cpp
@@ -151,7 +151,12 @@ void GraphingCalculator::OnEquationsVectorChanged(IObservableVector<EquationView
 
 void GraphingCalculator::OnTracePointChanged(Windows::Foundation::Point newPoint)
 {
-    TraceValue->Text = "(" + newPoint.X.ToString() + ", " + newPoint.Y.ToString() + ")";
+    wstringstream traceValueString;
+
+    // TODO: The below precision should ideally be dynamic based on the current scale of the graph.
+    traceValueString << "x=" << fixed << setprecision(1) << newPoint.X << ", y=" << fixed << setprecision(1) << newPoint.Y;
+
+    TraceValue->Text = ref new String(traceValueString.str().c_str());
 
     auto peer = FrameworkElementAutomationPeer::FromElement(TraceValue);
 
@@ -507,6 +512,8 @@ void GraphingCalculator::TraceValuePopup_SizeChanged(Object ^ sender, SizeChange
 
 void CalculatorApp::GraphingCalculator::ActiveTracing_Checked(Platform::Object ^ sender, Windows::UI::Xaml::RoutedEventArgs ^ e)
 {
+    GraphingControl->Focus(::FocusState::Programmatic);
+
     m_activeTracingKeyUpToken = Window::Current->CoreWindow->KeyUp +=
         ref new Windows::Foundation::TypedEventHandler<Windows::UI::Core::CoreWindow ^, Windows::UI::Core::KeyEventArgs ^>(
             this, &CalculatorApp::GraphingCalculator::ActiveTracing_KeyUp);
diff --git a/src/Calculator/Views/GraphingCalculator/KeyGraphFeaturesPanel.xaml b/src/Calculator/Views/GraphingCalculator/KeyGraphFeaturesPanel.xaml
index 0c43f60..0c3ffa7 100644
--- a/src/Calculator/Views/GraphingCalculator/KeyGraphFeaturesPanel.xaml
+++ b/src/Calculator/Views/GraphingCalculator/KeyGraphFeaturesPanel.xaml
@@ -25,7 +25,7 @@
             <Setter Property="Template">
                 <Setter.Value>
                     <ControlTemplate TargetType="controls:EquationTextBox">
-                        <Grid>
+                        <Grid Background="{ThemeResource TextControlBackground}">
 
                             <Grid.ColumnDefinitions>
                                 <ColumnDefinition Width="Auto"/>
@@ -102,7 +102,7 @@
             <Setter Property="IsReadOnly" Value="True"/>
             <Setter Property="Background" Value="Transparent"/>
             <Setter Property="IsEnabled" Value="True"/>
-            <Setter Property="Margin" Value="-8,0,0,0"/>
+            <Setter Property="Padding" Value="0,3,4,0"/>
             <Setter Property="UseSystemFocusVisuals" Value="True"/>
             <Setter Property="TextWrapping" Value="NoWrap"/>
             <Setter Property="HorizontalAlignment" Value="Stretch"/>
@@ -169,8 +169,8 @@
                             <Border x:Name="BorderElement"
                                     Grid.Row="1"
                                     Grid.RowSpan="1"
-                                    MinWidth="{ThemeResource TextControlThemeMinWidth}"
-                                    MinHeight="{ThemeResource TextControlThemeMinHeight}"
+                                    MinWidth="0"
+                                    MinHeight="0"
                                     Background="{TemplateBinding Background}"
                                     BorderBrush="{TemplateBinding BorderBrush}"
                                     BorderThickness="{TemplateBinding BorderThickness}"
@@ -224,6 +224,7 @@
         <Style x:Name="KGF_TextBlockStyle" TargetType="TextBlock">
             <Setter Property="FontWeight" Value="Normal"/>
             <Setter Property="FontSize" Value="14"/>
+            <Setter Property="IsTextSelectionEnabled" Value="True"/>
             <Setter Property="TextWrapping" Value="WrapWholeWords"/>
         </Style>
 
@@ -243,6 +244,11 @@
                            Style="{StaticResource KGF_TitleTextBlockStyle}"
                            Text="{x:Bind Title, Mode=OneWay}"/>
                 <ItemsControl ItemsSource="{x:Bind DisplayItems, Mode=OneWay}" UseSystemFocusVisuals="True">
+                    <ItemsControl.ItemContainerStyle>
+                        <Style TargetType="ContentPresenter">
+                            <Setter Property="Margin" Value="0"/>
+                        </Style>
+                    </ItemsControl.ItemContainerStyle>
                     <ItemsControl.ItemTemplate>
                         <DataTemplate x:DataType="x:String">
                             <controls:MathRichEditBox HorizontalAlignment="Left"
@@ -265,7 +271,7 @@
                         <DataTemplate x:DataType="vm:GridDisplayItems">
                             <Grid VerticalAlignment="Center">
                                 <Grid.ColumnDefinitions>
-                                    <ColumnDefinition Width="Auto"/>
+                                    <ColumnDefinition Width="Auto" MinWidth="64"/>
                                     <ColumnDefinition Width="Auto"/>
                                 </Grid.ColumnDefinitions>
                                 <controls:MathRichEditBox Grid.Column="0"
@@ -308,7 +314,7 @@
         <converters:BooleanToVisibilityNegationConverter x:Name="BooleanToVisibilityNegationConverter"/>
     </UserControl.Resources>
 
-    <Grid Margin="0,7,0,0" Background="Transparent">
+    <Grid Background="Transparent">
         <Grid.RowDefinitions>
             <RowDefinition Height="Auto"/>
             <RowDefinition Height="Auto"/>
@@ -335,8 +341,7 @@
 
         <ListView x:Name="KeyGraphFeaturesListView"
                   Grid.Row="2"
-                  Margin="12,10,10,0"
-                  Padding="0,0,0,12"
+                  Padding="12,10,10,12"
                   IsItemClickEnabled="False"
                   ItemContainerStyle="{StaticResource KGF_ListViewItemContainerStyle}"
                   ItemTemplateSelector="{StaticResource KGFTemplateSelector}"
@@ -350,6 +355,7 @@
                    Margin="12,10,10,0"
                    Style="{StaticResource KGF_TextBlockStyle}"
                    FontWeight="Normal"
+                   IsTextSelectionEnabled="False"
                    Text="{x:Bind ViewModel.AnalysisErrorString, Mode=OneWay}"
                    Visibility="{x:Bind ViewModel.AnalysisErrorVisible, Mode=OneWay}"/>
     </Grid>
diff --git a/src/Calculator/pch.h b/src/Calculator/pch.h
index add4efd..b56d026 100644
--- a/src/Calculator/pch.h
+++ b/src/Calculator/pch.h
@@ -27,6 +27,7 @@
 #include <concrt.h>
 #include <regex>
 #include <string>
+#include <iomanip>
 
 // C++\WinRT Headers
 #include "winrt/base.h"
diff --git a/src/GraphControl/Control/Grapher.cpp b/src/GraphControl/Control/Grapher.cpp
index 01756d2..764bed9 100644
--- a/src/GraphControl/Control/Grapher.cpp
+++ b/src/GraphControl/Control/Grapher.cpp
@@ -621,10 +621,9 @@ namespace GraphControl
 
     void Grapher::UpdateTracingChanged()
     {
-        if (m_renderMain->Tracing || m_renderMain->ActiveTracing)
+        if (m_renderMain->Tracing)
         {
             TracingChangedEvent(true);
-
             TracingValueChangedEvent(m_renderMain->TraceValue);
         }
         else
@@ -650,11 +649,7 @@ namespace GraphControl
         if (m_renderMain)
         {
             m_renderMain->DrawNearestPoint = false;
-            if (ActiveTracing == false)
-            {
-                // IF we are active tracing we never want to hide the popup..
-                TracingChangedEvent(false);
-            }
+            TracingChangedEvent(false);
             e->Handled = true;
         }
     }
diff --git a/src/GraphControl/DirectX/RenderMain.cpp b/src/GraphControl/DirectX/RenderMain.cpp
index a9b373d..e7fb053 100644
--- a/src/GraphControl/DirectX/RenderMain.cpp
+++ b/src/GraphControl/DirectX/RenderMain.cpp
@@ -35,7 +35,7 @@ namespace GraphControl::DX
         , m_backgroundColor{ {} }
         , m_swapChainPanel{ panel }
         , m_TraceValue(Point(0, 0))
-        , m_TraceLocation(Point(0,0))
+        , m_TraceLocation(Point(0, 0))
         , m_Tracing(false)
         , m_ActiveTracingPointRenderer{ &m_deviceResources }
     {
@@ -195,6 +195,14 @@ namespace GraphControl::DX
 
                         if (!isnan(nearestPointLocation.X) && !isnan(nearestPointLocation.Y))
                         {
+                            auto lineColors = m_graph->GetOptions().GetGraphColors();
+
+                            if (formulaId >= 0 && formulaId < lineColors.size())
+                            {
+                                auto dotColor = lineColors[formulaId];
+                                m_nearestPointRenderer.SetColor(D2D1::ColorF(dotColor.R * 65536 + dotColor.G * 256 + dotColor.B, 1.0));
+                            }
+
                             m_nearestPointRenderer.Render(nearestPointLocation);
                             m_Tracing = true;
                             m_TraceLocation = Point(nearestPointLocation.X, nearestPointLocation.Y);