From 712bdb1b7c1ad0689a4c555ccc84d6ed2c27c246 Mon Sep 17 00:00:00 2001 From: Rudy Huyn Date: Tue, 7 Jan 2020 11:03:47 -0800 Subject: [PATCH] Code cleaning: Remove all properties/functions not used in CalculationResult (#649) --- .../StandardCalculatorViewModel.h | 1 - src/Calculator/Controls/CalculationResult.cpp | 40 ++++++------------- src/Calculator/Controls/CalculationResult.h | 12 +++--- src/Calculator/Views/Calculator.xaml | 5 --- src/Calculator/Views/UnitConverter.xaml | 2 - .../StandardViewModelUnitTests.cpp | 33 +++++---------- 6 files changed, 28 insertions(+), 65 deletions(-) diff --git a/src/CalcViewModel/StandardCalculatorViewModel.h b/src/CalcViewModel/StandardCalculatorViewModel.h index d9c0ce3a..909e32c9 100644 --- a/src/CalcViewModel/StandardCalculatorViewModel.h +++ b/src/CalcViewModel/StandardCalculatorViewModel.h @@ -57,7 +57,6 @@ namespace CalculatorApp PROPERTY_R(Platform::String ^, SelectedExpressionLastData); OBSERVABLE_NAMED_PROPERTY_R(bool, IsInError); OBSERVABLE_PROPERTY_R(bool, IsOperatorCommand); - OBSERVABLE_PROPERTY_R(Platform::String ^, DisplayStringExpression); OBSERVABLE_PROPERTY_R(Windows::Foundation::Collections::IObservableVector ^, ExpressionTokens); OBSERVABLE_PROPERTY_R(Platform::String ^, DecimalDisplayValue); OBSERVABLE_PROPERTY_R(Platform::String ^, HexDisplayValue); diff --git a/src/Calculator/Controls/CalculationResult.cpp b/src/Calculator/Controls/CalculationResult.cpp index 844f1f50..ed092fab 100644 --- a/src/Calculator/Controls/CalculationResult.cpp +++ b/src/Calculator/Controls/CalculationResult.cpp @@ -26,16 +26,12 @@ using namespace Windows::UI::Xaml::Automation::Peers; using namespace std; DEPENDENCY_PROPERTY_INITIALIZATION(CalculationResult, IsActive); -DEPENDENCY_PROPERTY_INITIALIZATION(CalculationResult, AccentColor); DEPENDENCY_PROPERTY_INITIALIZATION(CalculationResult, MinFontSize); DEPENDENCY_PROPERTY_INITIALIZATION(CalculationResult, MaxFontSize); DEPENDENCY_PROPERTY_INITIALIZATION(CalculationResult, DisplayMargin); -DEPENDENCY_PROPERTY_INITIALIZATION(CalculationResult, MaxExpressionHistoryCharacters); -DEPENDENCY_PROPERTY_INITIALIZATION(CalculationResult, ExpressionVisibility); DEPENDENCY_PROPERTY_INITIALIZATION(CalculationResult, DisplayValue); DEPENDENCY_PROPERTY_INITIALIZATION(CalculationResult, IsInError); DEPENDENCY_PROPERTY_INITIALIZATION(CalculationResult, IsOperatorCommand); -DEPENDENCY_PROPERTY_INITIALIZATION(CalculationResult, DisplayStringExpression); #define SCALEFACTOR 0.357143 #define SMALLHEIGHTSCALEFACTOR 0 @@ -46,6 +42,7 @@ DEPENDENCY_PROPERTY_INITIALIZATION(CalculationResult, DisplayStringExpression); #define WIDTHTOFONTOFFSET 3 #define WIDTHCUTOFF 50 #define FONTTOLERANCE 0.001 +#define SCROLL_RATIO 0.7 // We need a safety margin to guarantee we correctly always show/hide ScrollLeft and ScrollRight buttons when necessary. // In rare cases, ScrollViewer::HorizontalOffset is a little low by a few (sub)pixels when users scroll to one of the extremity @@ -120,12 +117,12 @@ void CalculationResult::OnApplyTemplate() m_scrollLeft = dynamic_cast(GetTemplateChild("ScrollLeft")); if (m_scrollLeft) { - m_scrollLeftClickToken = m_scrollLeft->Click += ref new RoutedEventHandler(this, &CalculationResult::OnScrollClick); + m_scrollLeftClickToken = m_scrollLeft->Click += ref new RoutedEventHandler(this, &CalculationResult::OnScrollLeftClick); } m_scrollRight = dynamic_cast(GetTemplateChild("ScrollRight")); if (m_scrollRight) { - m_scrollRightClickToken = m_scrollRight->Click += ref new RoutedEventHandler(this, &CalculationResult::OnScrollClick); + m_scrollRightClickToken = m_scrollRight->Click += ref new RoutedEventHandler(this, &CalculationResult::OnScrollRightClick); } m_textBlock = dynamic_cast(GetTemplateChild("NormalOutput")); if (m_textBlock) @@ -156,16 +153,6 @@ void CalculationResult::OnIsActivePropertyChanged(bool /*oldValue*/, bool /*newV UpdateVisualState(); } -void CalculationResult::OnAccentColorPropertyChanged(Brush ^ /*oldValue*/, Brush ^ /*newValue*/) -{ - // Force the "Active" transition to happen again - if (IsActive) - { - VisualStateManager::GoToState(this, "Normal", true); - VisualStateManager::GoToState(this, "Active", true); - } -} - void CalculationResult::OnDisplayValuePropertyChanged(String ^ /*oldValue*/, String ^ /*newValue*/) { UpdateTextState(); @@ -287,7 +274,7 @@ void CalculationResult::ScrollLeft() } if (m_textContainer->HorizontalOffset > 0) { - double offset = m_textContainer->HorizontalOffset - (scrollRatio * m_textContainer->ViewportWidth); + double offset = m_textContainer->HorizontalOffset - (SCROLL_RATIO * m_textContainer->ViewportWidth); m_textContainer->ChangeView(offset, nullptr, nullptr); } } @@ -301,7 +288,7 @@ void CalculationResult::ScrollRight() if (m_textContainer->HorizontalOffset < m_textContainer->ExtentWidth - m_textContainer->ViewportWidth) { - double offset = m_textContainer->HorizontalOffset + (scrollRatio * m_textContainer->ViewportWidth); + double offset = m_textContainer->HorizontalOffset + (SCROLL_RATIO * m_textContainer->ViewportWidth); m_textContainer->ChangeView(offset, nullptr, nullptr); } } @@ -319,17 +306,14 @@ void CalculationResult::OnKeyDown(KeyRoutedEventArgs ^ e) } } -void CalculationResult::OnScrollClick(Object ^ sender, RoutedEventArgs ^ /*e*/) +void CalculationResult::OnScrollLeftClick(Object ^ sender, RoutedEventArgs ^ /*e*/) { - auto clicked = dynamic_cast(sender); - if (clicked == m_scrollLeft) - { - this->ScrollLeft(); - } - else - { - this->ScrollRight(); - } + ScrollLeft(); +} + +void CalculationResult::OnScrollRightClick(Object ^ sender, RoutedEventArgs ^ /*e*/) +{ + ScrollRight(); } void CalculationResult::UpdateScrollButtons() diff --git a/src/Calculator/Controls/CalculationResult.h b/src/Calculator/Controls/CalculationResult.h index b0297032..2db9fdbe 100644 --- a/src/Calculator/Controls/CalculationResult.h +++ b/src/Calculator/Controls/CalculationResult.h @@ -20,15 +20,11 @@ namespace CalculatorApp DEPENDENCY_PROPERTY_OWNER(CalculationResult); - DEPENDENCY_PROPERTY(Windows::UI::Xaml::Visibility, ExpressionVisibility); DEPENDENCY_PROPERTY_WITH_DEFAULT_AND_CALLBACK(double, MinFontSize, 0.0); DEPENDENCY_PROPERTY_WITH_DEFAULT_AND_CALLBACK(double, MaxFontSize, 30.0); DEPENDENCY_PROPERTY(Windows::UI::Xaml::Thickness, DisplayMargin); - DEPENDENCY_PROPERTY(int, MaxExpressionHistoryCharacters); DEPENDENCY_PROPERTY_WITH_CALLBACK(bool, IsActive); - DEPENDENCY_PROPERTY_WITH_CALLBACK(Windows::UI::Xaml::Media::Brush ^, AccentColor); DEPENDENCY_PROPERTY_WITH_CALLBACK(Platform::String ^, DisplayValue); - DEPENDENCY_PROPERTY(Platform::String ^, DisplayStringExpression); DEPENDENCY_PROPERTY_WITH_CALLBACK(bool, IsInError); DEPENDENCY_PROPERTY_WITH_DEFAULT(bool, IsOperatorCommand, false); @@ -48,7 +44,6 @@ namespace CalculatorApp private: void OnIsActivePropertyChanged(bool oldValue, bool newValue); - void OnAccentColorPropertyChanged(Windows::UI::Xaml::Media::Brush ^ oldValue, Windows::UI::Xaml::Media::Brush ^ newValue); void OnDisplayValuePropertyChanged(Platform::String ^ oldValue, Platform::String ^ newValue); void OnIsInErrorPropertyChanged(bool oldValue, bool newValue); void OnMinFontSizePropertyChanged(double oldValue, double newValue); @@ -58,7 +53,11 @@ namespace CalculatorApp void OnTextContainerLayoutUpdated(Object ^ sender, Object ^ e); void OnTextContainerOnViewChanged(Object ^ sender, Windows::UI::Xaml::Controls::ScrollViewerViewChangedEventArgs ^ e); void UpdateVisualState(); - void OnScrollClick(Platform::Object ^ sender, Windows::UI::Xaml::RoutedEventArgs ^ e); + void UpdateAllState(); + void OnScrollLeftClick(Platform::Object ^ sender, Windows::UI::Xaml::RoutedEventArgs ^ e); + void OnScrollRightClick(Platform::Object ^ sender, Windows::UI::Xaml::RoutedEventArgs ^ e); + void OnPointerEntered(Platform::Object ^ sender, Windows::UI::Xaml::Input::PointerRoutedEventArgs ^ e); + void OnPointerExited(Platform::Object ^ sender, Windows::UI::Xaml::Input::PointerRoutedEventArgs ^ e); void ModifyFontAndMargin(Windows::UI::Xaml::Controls::TextBlock ^ textBlock, double fontChange); void UpdateScrollButtons(); void ScrollLeft(); @@ -69,7 +68,6 @@ namespace CalculatorApp Windows::UI::Xaml::Controls::TextBlock ^ m_textBlock; Windows::UI::Xaml::Controls::HyperlinkButton ^ m_scrollLeft; Windows::UI::Xaml::Controls::HyperlinkButton ^ m_scrollRight; - double scrollRatio = 0.7; bool m_isScalingText; bool m_haveCalculatedMax; Windows::Foundation::EventRegistrationToken m_textContainerLayoutChangedToken; diff --git a/src/Calculator/Views/Calculator.xaml b/src/Calculator/Views/Calculator.xaml index 84692067..71dddefa 100644 --- a/src/Calculator/Views/Calculator.xaml +++ b/src/Calculator/Views/Calculator.xaml @@ -547,7 +547,6 @@ - @@ -558,7 +557,6 @@ - @@ -569,7 +567,6 @@ - @@ -615,9 +612,7 @@ AutomationProperties.Name="{x:Bind Model.CalculationResultAutomationName, Mode=OneWay}" ContextCanceled="OnContextCanceled" ContextRequested="OnContextRequested" - DisplayStringExpression="{x:Bind Model.DisplayStringExpression, Mode=OneWay}" DisplayValue="{x:Bind Model.DisplayValue, Mode=OneWay}" - ExpressionVisibility="Visible" IsInError="{x:Bind Model.IsInError, Mode=OneWay}" IsOperatorCommand="{x:Bind Model.IsOperatorCommand, Mode=OneWay}" TabIndex="1" diff --git a/src/Calculator/Views/UnitConverter.xaml b/src/Calculator/Views/UnitConverter.xaml index 0e15b928..21c64034 100644 --- a/src/Calculator/Views/UnitConverter.xaml +++ b/src/Calculator/Views/UnitConverter.xaml @@ -559,7 +559,6 @@ ContextCanceled="OnContextCanceled" ContextRequested="OnContextRequested" DisplayValue="{x:Bind Model.Value1, Mode=OneWay}" - ExpressionVisibility="Collapsed" FlowDirection="{x:Bind LayoutDirection}" IsActive="{Binding Value1Active, Mode=TwoWay}" KeyDown="OnValueKeyDown" @@ -610,7 +609,6 @@ ContextCanceled="OnContextCanceled" ContextRequested="OnContextRequested" DisplayValue="{x:Bind Model.Value2, Mode=OneWay}" - ExpressionVisibility="Collapsed" FlowDirection="{x:Bind LayoutDirection}" IsActive="{Binding Value2Active, Mode=TwoWay}" KeyDown="OnValueKeyDown" diff --git a/src/CalculatorUnitTests/StandardViewModelUnitTests.cpp b/src/CalculatorUnitTests/StandardViewModelUnitTests.cpp index 6bf7dff8..1f9d40d7 100644 --- a/src/CalculatorUnitTests/StandardViewModelUnitTests.cpp +++ b/src/CalculatorUnitTests/StandardViewModelUnitTests.cpp @@ -103,10 +103,6 @@ namespace CalculatorUnitTests { VERIFY_ARE_EQUAL(Platform::StringReference(currentItem->expectedPrimaryDisplay.c_str()), viewModel->DisplayValue); } - if (currentItem->expectedExpressions != L"N/A" && viewModel->DisplayStringExpression != nullptr) - { - VERIFY_ARE_EQUAL(Platform::StringReference(currentItem->expectedExpressions.c_str()), viewModel->DisplayStringExpression); - } currentItem++; } } @@ -122,19 +118,13 @@ namespace CalculatorUnitTests m_decimalSeparator = ref new Platform::String(m_engineResourceProvider->GetCEngineString(L"sDecimal").c_str()); } - void ValidateViewModelValueAndExpression(String ^ value, String ^ expression = nullptr) + void ValidateViewModelValue(String ^ value) { String ^ displayValue = m_viewModel->DisplayValue; - String ^ displayExpression = m_viewModel->DisplayStringExpression; if (value != nullptr) { VERIFY_ARE_EQUAL(value, displayValue); } - - if (expression != nullptr) - { - VERIFY_ARE_EQUAL(expression, displayExpression); - } } void ValidateViewModelValueAndSecondaryExpression(String ^ value, String ^ expression = nullptr) @@ -167,7 +157,6 @@ namespace CalculatorUnitTests StandardCalculatorViewModel ^ vmconstructortest = ref new StandardCalculatorViewModel(); vmconstructortest->IsStandard = true; String ^ displayValue = vmconstructortest->DisplayValue; - String ^ displayExpression = vmconstructortest->DisplayStringExpression; String ^ calculationResultAutomationName = vmconstructortest->CalculationResultAutomationName; VERIFY_ARE_EQUAL(StringReference(L"0"), displayValue); @@ -401,18 +390,18 @@ namespace CalculatorUnitTests m_viewModel->IsScientific = false; m_viewModel->OnPaste("-0.99"); - ValidateViewModelValueAndExpression("-0" + m_decimalSeparator + "99", ""); + ValidateViewModelValue("-0" + m_decimalSeparator + "99"); m_viewModel->OnPaste("1+1="); - ValidateViewModelValueAndExpression("2", ""); + ValidateViewModelValue("2"); // This result is not obvious: it's the result of the previous operation m_viewModel->OnPaste("0="); - ValidateViewModelValueAndExpression("1", ""); + ValidateViewModelValue("1"); // Negative value m_viewModel->OnPaste("-1"); - ValidateViewModelValueAndExpression("-1", ""); + ValidateViewModelValue("-1"); // Negated expression m_viewModel->OnPaste("-(1+1)"); @@ -429,24 +418,24 @@ namespace CalculatorUnitTests //// Positive exponent m_viewModel->OnPaste("1.23e+10"); - ValidateViewModelValueAndExpression("1" + m_decimalSeparator + "23e+10", ""); + ValidateViewModelValue("1" + m_decimalSeparator + "23e+10"); m_viewModel->OnPaste("1.23e10"); - ValidateViewModelValueAndExpression("1" + m_decimalSeparator + "23e+10", ""); + ValidateViewModelValue("1" + m_decimalSeparator + "23e+10"); m_viewModel->OnPaste("135e10"); - ValidateViewModelValueAndExpression("135" + m_decimalSeparator + "e+10", ""); + ValidateViewModelValue("135" + m_decimalSeparator + "e+10"); //// Negative exponent m_viewModel->OnPaste("1.23e-10"); - ValidateViewModelValueAndExpression("1" + m_decimalSeparator + "23e-10", ""); + ValidateViewModelValue("1" + m_decimalSeparator + "23e-10"); //// Uppercase E (for exponent) m_viewModel->OnPaste("1.23E-10"); - ValidateViewModelValueAndExpression("1" + m_decimalSeparator + "23e-10", ""); + ValidateViewModelValue("1" + m_decimalSeparator + "23e-10"); m_viewModel->OnPaste("135E10"); - ValidateViewModelValueAndExpression("135" + m_decimalSeparator + "e+10", ""); + ValidateViewModelValue("135" + m_decimalSeparator + "e+10"); } // Verify Calculator CalculationResultAutomationName is set correctly