From 319ac1ff12c330b45bcbb185a5ac731cb2da59e3 Mon Sep 17 00:00:00 2001 From: Rudy Huyn Date: Thu, 28 Mar 2019 02:06:45 -0700 Subject: [PATCH] - remove double scrollviewer - optimize ExpressionTokens in StandardCalculatorViewModel to only update new tokens instead of the full list - refactor how we manage the visibility of left/right buttons - remove useless ExpressionItemContainerStyle - only modify accessbility view if necessary --- .../StandardCalculatorViewModel.cpp | 106 ++++++++------- .../StandardCalculatorViewModel.h | 2 +- src/Calculator/Calculator.vcxproj | 2 - src/Calculator/Calculator.vcxproj.filters | 6 - src/Calculator/Controls/OverflowTextBlock.cpp | 78 +++++------ src/Calculator/Controls/OverflowTextBlock.h | 6 +- .../ExpressionItemContainerStyle.cpp | 44 ------ .../Converters/ExpressionItemContainerStyle.h | 71 ---------- src/Calculator/Views/Calculator.xaml | 126 +++++++----------- src/Calculator/Views/Calculator.xaml.cpp | 5 - src/Calculator/Views/Calculator.xaml.h | 4 +- 11 files changed, 141 insertions(+), 309 deletions(-) delete mode 100644 src/Calculator/Converters/ExpressionItemContainerStyle.cpp delete mode 100644 src/Calculator/Converters/ExpressionItemContainerStyle.h diff --git a/src/CalcViewModel/StandardCalculatorViewModel.cpp b/src/CalcViewModel/StandardCalculatorViewModel.cpp index c01c4789..9bc479d4 100644 --- a/src/CalcViewModel/StandardCalculatorViewModel.cpp +++ b/src/CalcViewModel/StandardCalculatorViewModel.cpp @@ -129,6 +129,7 @@ StandardCalculatorViewModel::StandardCalculatorViewModel() : AreHistoryShortcutsEnabled = true; AreProgrammerRadixOperatorsEnabled = false; + m_ExpressionTokens = ref new Vector(); m_tokenPosition = -1; m_isLastOperationHistoryLoad = false; } @@ -321,59 +322,66 @@ void StandardCalculatorViewModel::SetTokens(_Inout_ shared_ptr(); - } - else - { - m_ExpressionTokens->Clear(); - } - unsigned int nTokens = 0; tokens->GetSize(&nTokens); + + if (nTokens == 0) + { + m_ExpressionTokens->Clear(); + return; + } + pair currentToken; const auto& localizer = LocalizationSettings::GetInstance(); + const wstring separator = L" "; for (unsigned int i = 0; i < nTokens; ++i) { if (SUCCEEDED(tokens->GetAt(i, ¤tToken))) { Common::TokenType type; - const wstring separator = L" "; bool isEditable = (currentToken.second == -1) ? false : true; localizer.LocalizeDisplayValue(&(currentToken.first)); if (!isEditable) { - if (currentToken.first == separator) - { - type = TokenType::Separator; - } - else - { - type = TokenType::Operator; - } + type = currentToken.first == separator ? TokenType::Separator : TokenType::Operator; } - else { shared_ptr command; IFTPlatformException(m_commands->GetAt(static_cast(currentToken.second), &command)); + type = command->GetCommandType() == CommandType::OperandCommand ? TokenType::Operand : TokenType::Operator; + } - if (command->GetCommandType() == CommandType::OperandCommand) + auto currentTokenString = ref new String(currentToken.first.c_str()); + auto expressionToken = ref new DisplayExpressionToken(currentTokenString, i, isEditable, type); + if (i < m_ExpressionTokens->Size) + { + auto existingItem = m_ExpressionTokens->GetAt(i); + if (type == existingItem->Type && expressionToken->Token->Equals(currentTokenString)) { - type = TokenType::Operand; + existingItem->TokenPosition = i; + existingItem->IsTokenEditable = isEditable; + existingItem->CommandIndex = expressionToken->CommandIndex; } else { - type = TokenType::Operator; + m_ExpressionTokens->InsertAt(i, expressionToken); } + + } + else + { + m_ExpressionTokens->Append(expressionToken); } - DisplayExpressionToken^ expressionToken = ref new DisplayExpressionToken(ref new String(currentToken.first.c_str()), i, isEditable, type); - m_ExpressionTokens->Append(expressionToken); } } + + while (m_ExpressionTokens->Size != nTokens) + { + m_ExpressionTokens->RemoveAtEnd(); + } } String^ StandardCalculatorViewModel::GetCalculatorExpressionAutomationName() @@ -531,7 +539,7 @@ void StandardCalculatorViewModel::HandleUpdatedOperandData(Command cmdenum) { if (commandIndex == 0) { - delete [] temp; + delete[] temp; return; } @@ -552,7 +560,7 @@ void StandardCalculatorViewModel::HandleUpdatedOperandData(Command cmdenum) length = m_selectedExpressionLastData->Length() + 1; if (length > 50) { - delete [] temp; + delete[] temp; return; } for (; i < length; ++i) @@ -1422,29 +1430,29 @@ void StandardCalculatorViewModel::SaveEditedCommand(_In_ unsigned int tokenPosit switch (nOpCode) { - case static_cast(Command::CommandASIN) : - updatedToken = CCalcEngine::OpCodeToUnaryString(static_cast(Command::CommandSIN), true, angleType); - break; - case static_cast(Command::CommandACOS) : - updatedToken = CCalcEngine::OpCodeToUnaryString(static_cast(Command::CommandCOS), true, angleType); - break; - case static_cast(Command::CommandATAN) : - updatedToken = CCalcEngine::OpCodeToUnaryString(static_cast(Command::CommandTAN), true, angleType); - break; - case static_cast(Command::CommandASINH) : - updatedToken = CCalcEngine::OpCodeToUnaryString(static_cast(Command::CommandSINH), true, angleType); - break; - case static_cast(Command::CommandACOSH) : - updatedToken = CCalcEngine::OpCodeToUnaryString(static_cast(Command::CommandCOSH), true, angleType); - break; - case static_cast(Command::CommandATANH) : - updatedToken = CCalcEngine::OpCodeToUnaryString(static_cast(Command::CommandTANH), true, angleType); - break; - case static_cast(Command::CommandPOWE) : - updatedToken = CCalcEngine::OpCodeToUnaryString(static_cast(Command::CommandLN), true, angleType); - break; - default: - updatedToken = CCalcEngine::OpCodeToUnaryString(nOpCode, false, angleType); + case static_cast(Command::CommandASIN) : + updatedToken = CCalcEngine::OpCodeToUnaryString(static_cast(Command::CommandSIN), true, angleType); + break; + case static_cast(Command::CommandACOS) : + updatedToken = CCalcEngine::OpCodeToUnaryString(static_cast(Command::CommandCOS), true, angleType); + break; + case static_cast(Command::CommandATAN) : + updatedToken = CCalcEngine::OpCodeToUnaryString(static_cast(Command::CommandTAN), true, angleType); + break; + case static_cast(Command::CommandASINH) : + updatedToken = CCalcEngine::OpCodeToUnaryString(static_cast(Command::CommandSINH), true, angleType); + break; + case static_cast(Command::CommandACOSH) : + updatedToken = CCalcEngine::OpCodeToUnaryString(static_cast(Command::CommandCOSH), true, angleType); + break; + case static_cast(Command::CommandATANH) : + updatedToken = CCalcEngine::OpCodeToUnaryString(static_cast(Command::CommandTANH), true, angleType); + break; + case static_cast(Command::CommandPOWE) : + updatedToken = CCalcEngine::OpCodeToUnaryString(static_cast(Command::CommandLN), true, angleType); + break; + default: + updatedToken = CCalcEngine::OpCodeToUnaryString(nOpCode, false, angleType); } if ((token.first.length() > 0) && (token.first[token.first.length() - 1] == L'(')) { diff --git a/src/CalcViewModel/StandardCalculatorViewModel.h b/src/CalcViewModel/StandardCalculatorViewModel.h index ba6dc82b..5a2dbff9 100644 --- a/src/CalcViewModel/StandardCalculatorViewModel.h +++ b/src/CalcViewModel/StandardCalculatorViewModel.h @@ -48,7 +48,7 @@ namespace CalculatorApp OBSERVABLE_NAMED_PROPERTY_RW(bool, IsInError); OBSERVABLE_PROPERTY_RW(bool, IsOperatorCommand); OBSERVABLE_PROPERTY_RW(Platform::String^, DisplayStringExpression); - OBSERVABLE_PROPERTY_RW(Windows::Foundation::Collections::IVector^, ExpressionTokens); + OBSERVABLE_PROPERTY_R(Windows::Foundation::Collections::IObservableVector^, ExpressionTokens); OBSERVABLE_PROPERTY_RW(Platform::String^, DecimalDisplayValue); OBSERVABLE_PROPERTY_RW(Platform::String^, HexDisplayValue); OBSERVABLE_PROPERTY_RW(Platform::String^, OctalDisplayValue); diff --git a/src/Calculator/Calculator.vcxproj b/src/Calculator/Calculator.vcxproj index 4e19aab2..a12067d4 100644 --- a/src/Calculator/Calculator.vcxproj +++ b/src/Calculator/Calculator.vcxproj @@ -246,7 +246,6 @@ - @@ -380,7 +379,6 @@ - diff --git a/src/Calculator/Calculator.vcxproj.filters b/src/Calculator/Calculator.vcxproj.filters index cc25a224..d87d8524 100644 --- a/src/Calculator/Calculator.vcxproj.filters +++ b/src/Calculator/Calculator.vcxproj.filters @@ -243,9 +243,6 @@ Converters - - Converters - Converters @@ -342,9 +339,6 @@ Converters - - Converters - Converters diff --git a/src/Calculator/Controls/OverflowTextBlock.cpp b/src/Calculator/Controls/OverflowTextBlock.cpp index 85baebc4..a0f5e705 100644 --- a/src/Calculator/Controls/OverflowTextBlock.cpp +++ b/src/Calculator/Controls/OverflowTextBlock.cpp @@ -34,7 +34,7 @@ void OverflowTextBlock::OnApplyTemplate() m_expressionContainer = safe_cast(GetTemplateChild("expressionContainer")); m_expressionContainer->ChangeView(m_expressionContainer->ExtentWidth - m_expressionContainer->ViewportWidth, nullptr, nullptr); - + m_expressionContainer->ViewChanged += ref new Windows::Foundation::EventHandler(this, &CalculatorApp::Controls::OverflowTextBlock::OnViewChanged); m_scrollLeft = safe_cast(GetTemplateChild("scrollLeft")); m_scrollRight = safe_cast(GetTemplateChild("scrollRight")); @@ -44,12 +44,7 @@ void OverflowTextBlock::OnApplyTemplate() m_scrollingLeft = false; m_scrollingRight = false; - auto borderContainer = safe_cast(GetTemplateChild("expressionborder")); - m_pointerEnteredEventToken = borderContainer->PointerEntered += ref new PointerEventHandler(this, &OverflowTextBlock::OnPointerEntered); - m_pointerExitedEventToken = borderContainer->PointerExited += ref new PointerEventHandler(this, &OverflowTextBlock::OnPointerExited); - - m_listView = safe_cast(GetTemplateChild("TokenList")); - + m_itemsControl = safe_cast(GetTemplateChild("TokenList")); UpdateAllState(); } @@ -60,18 +55,19 @@ AutomationPeer^ OverflowTextBlock::OnCreateAutomationPeer() void OverflowTextBlock::OnTokensUpdatedPropertyChanged(bool /*oldValue*/, bool newValue) { - if ((m_listView != nullptr) && (newValue)) + if ((m_expressionContainer != nullptr) && (newValue)) { - unsigned int tokenCount = m_listView->Items->Size; - if (tokenCount > 0) - { - m_listView->UpdateLayout(); - m_listView->ScrollIntoView(m_listView->Items->GetAt(tokenCount - 1)); - m_expressionContainer->ChangeView(m_expressionContainer->ExtentWidth - m_expressionContainer->ViewportWidth, nullptr, nullptr); - } + m_expressionContainer->UpdateLayout(); + m_expressionContainer->ChangeView(m_expressionContainer->ScrollableWidth, nullptr, nullptr, true); } - AutomationProperties::SetAccessibilityView(this, - m_listView != nullptr && m_listView->Items->Size > 0 ? AccessibilityView::Control : AccessibilityView::Raw); + auto newIsAccessibilityViewControl = m_itemsControl != nullptr && m_itemsControl->Items->Size > 0; + if (m_isAccessibilityViewControl != newIsAccessibilityViewControl) + { + m_isAccessibilityViewControl = newIsAccessibilityViewControl; + AutomationProperties::SetAccessibilityView(this, + newIsAccessibilityViewControl ? AccessibilityView::Control : AccessibilityView::Raw); + } + UpdateScrollButtons(); } void OverflowTextBlock::UpdateAllState() @@ -128,26 +124,15 @@ void OverflowTextBlock::OnScrollClick(_In_ Object^ sender, _In_ RoutedEventArgs^ } } -void OverflowTextBlock::OnPointerEntered(_In_ Object^, _In_ PointerRoutedEventArgs^ e) -{ - if (e->Pointer->PointerDeviceType == PointerDeviceType::Mouse) - { - UpdateScrollButtons(); - } -} - -void OverflowTextBlock::OnPointerExited(_In_ Object^, _In_ PointerRoutedEventArgs^ e) -{ - if (e->Pointer->PointerDeviceType == PointerDeviceType::Mouse) - { - UpdateScrollButtons(); - } -} - void OverflowTextBlock::UpdateScrollButtons() { + if (m_itemsControl == nullptr && m_expressionContainer == nullptr) + { + return; + } + // When the width is smaller than the container, don't show any - if (m_listView->ActualWidth <= m_expressionContainer->ActualWidth) + if (m_itemsControl->ActualWidth <= m_expressionContainer->ActualWidth) { ShowHideScrollButtons(::Visibility::Collapsed, ::Visibility::Collapsed); } @@ -163,7 +148,10 @@ void OverflowTextBlock::UpdateScrollButtons() if (m_scrollingLeft) { m_scrollingLeft = false; - m_scrollRight->Focus(::FocusState::Programmatic); + if (m_scrollRight != nullptr) + { + m_scrollRight->Focus(::FocusState::Programmatic); + } } } else // Width is larger than the container and right most part of the number is shown. Should be able to scroll left. @@ -172,7 +160,10 @@ void OverflowTextBlock::UpdateScrollButtons() if (m_scrollingRight) { m_scrollingRight = false; - m_scrollLeft->Focus(::FocusState::Programmatic); + if (m_scrollLeft != nullptr) + { + m_scrollLeft->Focus(::FocusState::Programmatic); + } } } } @@ -198,13 +189,10 @@ void OverflowTextBlock::UnregisterEventHandlers() { m_scrollRight->Click -= m_scrollRightClickEventToken; } - - auto borderContainer = safe_cast(GetTemplateChild("expressionborder")); - - // Adding an extra check, in case the returned template is null - if (borderContainer != nullptr) - { - borderContainer->PointerEntered -= m_pointerEnteredEventToken; - borderContainer->PointerExited -= m_pointerExitedEventToken; - } +} + + +void CalculatorApp::Controls::OverflowTextBlock::OnViewChanged(Platform::Object ^sender, Windows::UI::Xaml::Controls::ScrollViewerViewChangedEventArgs ^args) +{ + UpdateScrollButtons(); } diff --git a/src/Calculator/Controls/OverflowTextBlock.h b/src/Calculator/Controls/OverflowTextBlock.h index 13b2e828..d155475a 100644 --- a/src/Calculator/Controls/OverflowTextBlock.h +++ b/src/Calculator/Controls/OverflowTextBlock.h @@ -44,15 +44,15 @@ namespace CalculatorApp double scrollRatio = 0.7; bool m_scrollingLeft; bool m_scrollingRight; - Windows::UI::Xaml::Controls::ListView^ m_listView; + bool m_isAccessibilityViewControl; + Windows::UI::Xaml::Controls::ItemsControl^ m_itemsControl; Windows::UI::Xaml::Controls::ScrollViewer^ m_expressionContainer; Windows::UI::Xaml::Controls::Button^ m_scrollLeft; Windows::UI::Xaml::Controls::Button^ m_scrollRight; Windows::Foundation::EventRegistrationToken m_scrollLeftClickEventToken; Windows::Foundation::EventRegistrationToken m_scrollRightClickEventToken; - Windows::Foundation::EventRegistrationToken m_pointerEnteredEventToken; - Windows::Foundation::EventRegistrationToken m_pointerExitedEventToken; + void OnViewChanged(Platform::Object ^sender, Windows::UI::Xaml::Controls::ScrollViewerViewChangedEventArgs ^args); }; } } diff --git a/src/Calculator/Converters/ExpressionItemContainerStyle.cpp b/src/Calculator/Converters/ExpressionItemContainerStyle.cpp deleted file mode 100644 index d99332c1..00000000 --- a/src/Calculator/Converters/ExpressionItemContainerStyle.cpp +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -#include "pch.h" -#include "ExpressionItemContainerStyle.h" -#include "CalcViewModel/Common/DisplayExpressionToken.h" - -using namespace CalculatorApp::Common; - -namespace CalculatorApp -{ - namespace Converters - { - Windows::UI::Xaml::Style^ ExpressionItemContainerStyle::SelectStyleCore(Platform::Object^ item, Windows::UI::Xaml::DependencyObject^ container) - { - DisplayExpressionToken^ token = dynamic_cast(item); - if (token != nullptr) - { - Common::TokenType type = token->Type; - - switch (type) - { - case TokenType::Operator: - if (token->IsTokenEditable) - { - return m_editableOperatorStyle; - } - else - { - return m_nonEditableOperatorStyle; - } - case TokenType::Operand: - return m_operandStyle; - case TokenType::Separator: - return m_separatorStyle; - default: - throw ref new Platform::Exception(E_FAIL, L"Invalid token type"); - } - } - - return m_separatorStyle; - } - } -} diff --git a/src/Calculator/Converters/ExpressionItemContainerStyle.h b/src/Calculator/Converters/ExpressionItemContainerStyle.h deleted file mode 100644 index e42ecc6a..00000000 --- a/src/Calculator/Converters/ExpressionItemContainerStyle.h +++ /dev/null @@ -1,71 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -#pragma once - -namespace CalculatorApp -{ - namespace Converters - { - [Windows::UI::Xaml::Data::Bindable] - public ref class ExpressionItemContainerStyle sealed : public Windows::UI::Xaml::Controls::StyleSelector - { - public: - virtual Windows::UI::Xaml::Style^ SelectStyleCore(Platform::Object^ item, Windows::UI::Xaml::DependencyObject^ container) override; - - property Windows::UI::Xaml::Style^ EditableOperatorStyle - { - Windows::UI::Xaml::Style^ get() - { - return m_editableOperatorStyle; - } - void set(Windows::UI::Xaml::Style^ val) - { - m_editableOperatorStyle = val; - } - } - - property Windows::UI::Xaml::Style^ OperandStyle - { - Windows::UI::Xaml::Style^ get() - { - return m_operandStyle; - } - void set(Windows::UI::Xaml::Style^ val) - { - m_operandStyle = val; - } - } - - property Windows::UI::Xaml::Style^ SeparatorStyle - { - Windows::UI::Xaml::Style^ get() - { - return m_separatorStyle; - } - void set(Windows::UI::Xaml::Style^ val) - { - m_separatorStyle = val; - } - } - - property Windows::UI::Xaml::Style^ NonEditableOperatorStyle - { - Windows::UI::Xaml::Style^ get() - { - return m_nonEditableOperatorStyle; - } - void set(Windows::UI::Xaml::Style^ val) - { - m_nonEditableOperatorStyle = val; - } - } - - private: - Windows::UI::Xaml::Style^ m_editableOperatorStyle; - Windows::UI::Xaml::Style^ m_nonEditableOperatorStyle; - Windows::UI::Xaml::Style^ m_operandStyle; - Windows::UI::Xaml::Style^ m_separatorStyle; - }; - } -} diff --git a/src/Calculator/Views/Calculator.xaml b/src/Calculator/Views/Calculator.xaml index 01164b83..a5f139f4 100644 --- a/src/Calculator/Views/Calculator.xaml +++ b/src/Calculator/Views/Calculator.xaml @@ -8,7 +8,6 @@ xmlns:d="http://schemas.microsoft.com/expression/blend/2008" xmlns:local="using:CalculatorApp" xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" - xmlns:model="using:CalculatorApp.ViewModel" Loaded="OnLoaded" mc:Ignorable="d"> @@ -19,20 +18,20 @@ + Text="{x:Bind Token}"/> + Text="{x:Bind Token}"/> + Text="{x:Bind Token}"/> @@ -141,66 +140,54 @@ - - - - - - - - - + + + + + + + - - - - - - - - - - - - - - + ItemsSource="{Binding ExpressionTokens}"> + + + + + + + + + + @@ -239,21 +226,6 @@ - - - -