Merge master into feature/GraphingCalculator branch #660
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merge with master
Merge with master
Description of the changes: Currently Calculator handles strings by defining integers for each type of function that can be performed, this integer will eventually correspond with an index in s_engineStrings which holds the corresponding display string for each function. Some functions such as Sin can have multiple strings (degrees, rads, grads, inverse). Functions like Sin are mapped to another array called "rgUfne" where a new integer is given depending on the output string which will then be given to s_engineStrings. The new integer returned by the "rgUfne" array runs the risk of overlapping with any new functions that may be added in CCommand.h. Furthermore, it is expected that the strings in s_engineStrings and rgUfne are defined in a particular order (not necessarily sequential), otherwise the logic will break. This makes adding new strings for new functions confusing and difficult, since a lot of the logic is not clearly defined. This PR attempts to make this a bit simpler by changing the s_engineStrings and rgUfne arrays to be unordered_maps instead of arrays. For s_engineStrings the keys will now be strings, allowing the existing logic for indexing to be used by simply converting the number into a string to access the value. This will also allow us to create keys in the future that are not limited to integers but to strings that hold more meaning. The rgUfne array will also be updated to be a map that will take in an integer and give you the corresponding string that can be passed to s_engineStrings. The UFNE object in the rgUfne array will also be updated to hold all the possible string keys for a function, instead of indexing them on other numbers that may overlap with existing definitions. Now to add a new string for a new IDC_FOO function, we would just need to add the "FooString" resource keys to the g_sids array and use the updated rgUfne map to link the IDC_FOO value to the corresponding "FooString" resource key. This way the resource key can be a meaningful string, and not an integer that must be in any particular order. How changes were validated: Tested each function manually in standard, scientific, and programmer modes.
Simplify some of the calc engine string logic (microsoft#449)
* Modify how we manage Narrator with parenthesis and refactor right parenthesis * Optimization * remove extra spaces * take feedback into account
…ation and users paste a text (microsoft#391) Fix the else condition in ApplicationViewModel::OnPaste How changes were validated: Manually Fixes microsoft#389
Fixes microsoft#260 Description of the changes: prevent UnitConverterViewModel to reset values when users click on update rates. recompute UnitConverter's caches (m_ratioMap and m_categoryToUnits) once rates are updated (but check first if the user did/didn't change the category) How changes were validated: Manually tested with fake currency rates (HTTP responses modified on the fly via FiddlerCore) Verified that it works no matter the selected field (From or To) Verified that the currencies selected are kept after a refresh
…soft#412) ## Fixes microsoft#111 > The modulo operator on this calculator gives the result that is different to the most used calculators. The current `modrate` function is the equivalent of rem(...)/remainder(...), not mod(...)/modulo(...) available in some popular Math apps. ### Description of the changes: - rename `modrate` in `remrate` to be more accurate. - add `modrate`, calculating modulo similarly to Matlab, Bing, Google calculator, Maxima, Wolfram Alpha and Microsoft Excel - Add `RationalMath::Mod` using `modrate` as an alternative to `Rational::operator%` using `remrate` - Add a helper `SIGN` to retrieve the sign of a `Rational`. - modify `CalcEngine` to use `modrate` in Normal and Scientific mode and `remrate` in Programmer mode. ### How changes were validated: - manually and unit tests added
…t#440) Fixes microsoft#407 Removed AppBar, OperatorTextBox and OperandTextBox controls
Fixes microsoft#382 Description of the changes: Add Pyeong as an Area conversion unit. Pyeong shows up only if the user's current region is Korea ( i.e. region is either KP or KR ). Added Korean translation for Pyeong (평). For other locales, we default to English ( Pyeong ). How changes were validated: Manually tested the below For non-Korean regions, Pyeong does not show up. Korean region with Korean locale => Pyeong shows up and Pyeong is correctly translated. pyeong_Korean Korean region with English locale => Pyeong shows up and Pyeong is in English. pyeong_English Korean region with simplified Chinese locale => Pyeong shows up and Pyeong is in English. pyeong_Chinese
…osoft#436) Fixes microsoft#324 . Description of the changes: In an effort to support other compilers (microsoft#109), this change reworks how precompiled headers are handled. For toolchains where precompiled headers are not used, there is unnecessary compilation cost because each source file explicity includes the pch, meaning all system headers in the pch were recompiled for each translation unit. This change modifies the project's files so that each translation unit includes a minimal set of dependent headers. For MSVC users, the precompiled headers option is still enabled and the precompiled header is added to each translation unit using the compiler's Forced Includes option. The end result is that MSVC users still see the same build times, but other toolchains are free to use or not use precompiled headers. Risks introduced Given that our CI build uses MSVC, this change introduces the risk that a system header is added to the pch and the CalcManager project builds correctly, but builds could be broken for other toolsets that don't use pch. We know we want to add support for Clang in our CI build (microsoft#211). It seems reasonable to also compile without precompiled headers there so that we can regression test this setup. How changes were validated: Rebuild CalcManager project. Compile time: ~4.5s. Disable precompiled headers, keeping explicit include for pch in each source file. Compile time: ~13s. Remove explicit pch inclusion and add the appropriate headers to each translation unit to allow the project to compile. Compile time: ~8s. Re-enable pch and include it using the Forced Includes compiler option. MSVC compile time: ~4.5s. Minor changes Delete 'targetver.h'. I found this while looking around for system headers in the project. It's unused and unreferenced so let's remove it.
Fixes microsoft#402 and microsoft#414 Divide by 4 the CPU usage of OverflowTextBlock when buttons are pressed very quickly. Description of the changes: Xaml-side: OverflowTextBlock has some performance issues: double scrollviewer: the listview was in a scrollviewer, while the control already containing one -> it breaks the virtualization of the listview and impacts on UI performance. The listview used a StackPanel, this panel doesn't support virtualization of ListViewItems contrary to ItemsStackPanel No ListView-specific features were used, an ItemsControl is more efficient and lighter. refactor how we manage the visibility of the left/right buttons in OverflowTextBlock, the new version is more reactive and will not display the right arrow when not necessary (see GIF below). remove the ItemContainerSelector ExpressionItemContainerStyle, not really used by OverflowTextBlock remove UI glitches generated by ChangeView when users type fast (control partially hidden and scrolling issues, see the GIF below). only modify the accessibility view when it's necessary ViewModel-side: stop fully refreshing ExpressionTokens in StandardCalculatorViewModel when a new command were sent, instead, use a IObservableVector to only send new tokens to the UI (in average only 1 or 2 UI items are refreshed while the full expression was refreshed before) How changes were validated: Manually
Description of the changes: -Refactored all x:Names to use the generally accepted Pascal-Casing How changes were validated: Unit Tests Manual Tests
Fixes microsoft#175. Fixes loop in test code to verify that commands not supported by the unit converter viewmodel result in no-op. Description of the changes: Removed loop of range of enums with a being tested for no-ops since it added no intrinsic value (and the way that the range was handled was incorrect). Considered adding an iterator over a static list of commands to validate against, but determined it didn't add any notable value. How changes were validated: Ran modified test to ensure it passes
Fix microsoft#409 - Some content in Currency Converter not right-aligned properly in RtL Fix microsoft#59 Currency symbol precedence is opposite of system setting in RTL languages Description of the changes: Add a property FlowDirectionHorizontalAlignment in UnitConverter to align some controls to the right (without modifying the FlowDirection of their parent items) Force FlowDirection of Value1Container and Value2Container to LeftToRight (but align panels to the right) How changes were validated: Tested with LtR and RtL languages and with currency symbols on the left and on the right.
Fixes microsoft#407 (partially) and microsoft#441 Description of the changes: Remove TitleBarHelper and all <Border x:Name="CustomTitleBar" /> Let the system defines the draggable region Centralize all events and functions associated to the title bar in a single control TitleBar instead of code splitted between MainPage/TitleBar/HistoryList/Memory. Use the standard title bar when high contrast is activated instead of the custom one. Modify the color of the title when the window doesn't have focus Fix the right padding of the title bar with high contrast How changes were validated: Manually tested with LtR and RtL languages Manually tested with high contrast Tested when History and Memory flyout are opened
…t view, like the menu items do (microsoft#447) Fixes microsoft#437. Clicking on the same element in the hamburger view should re-open that view, like the menu items do Description of the changes: -Fixed the bug that was listed How changes were validated: -manual
Related to microsoft#55 and microsoft#64 Description of the changes: Added constexpr to formerly static const or #define variables Applied C++ Core Guideline NR.2 Added auto and const in appropriate places How changes were validated: Used the provided unit tests
…block (microsoft#467) Fixes microsoft#313 In Scan/Item mode, Narrator focus navigates to hidden element “No next item” after “Update rates” link in "Currency Converter" window microsoft#313 Description of the changes: Adds an x:Name to the CurrencySecondaryStatus text block Adds a NormalCurrencyStatus visual state to the CurrencySecondaryStatusStates Adds functionality to the CurrencySecondaryStatusStates to show or hide the CurrencySecondaryStatus text block. How changes were validated: Verified that the textblock is not visible in the accessibility tree via inspect.exe from the windows sdk. Verified that Narrator also does not stop on the block when in scan mode. Verified that the textblock is visible in the accessibility tree and read out in Narrator when the ChargesMayApplyCurrencyStatus or FailedCurrencyStatus viewstates are set.
Fixes microsoft#119 Added infinite precision as a feature in README.md and in ApplicationArchitecture.md How changes were validated: Manual preview of README and ApplicationArchitecture.md
…ft#472) Description of the changes: Update the .gitignore to ignore GraphingImplOverrides.props. This update already exists in the feature branch but we can merge it to master now to make development of the feature easier and prevent accidentally commiting this file. How changes were validated: Manual. Switch to feature branch. Add GraphingImplOverrides.props Switch to dabelc/IgnoreGraphingImplOverrides.props. Confirm git shows no files changed but props file is present.
Description of the changes: This will cause Git to insert conflict markers in the file and make it easier to identify and fix merge conflicts. Previously, Git would complain about conflicts with these files, but no markers were inserted. How changes were validated: Manual. Checkout feature/GraphingCalculator and merge in upstream/master. Git identifies merge conflicts present for Calculator.vcxproj but there are no conflict markers in the file. Abort the merge. Merge in dabelc/MergeProjectFilesAsText. Confirm Git is able to merge Calculator.vcxproj.
[JaEra] Calc: subtracting 1-year from date during Reiwa 1 yields unexpected results.
Merge with master
* Ignore None characters while parsing the clipboard * reformat
There is an odd dependency in CalcEngine. `CalculatorManager` inherits `ICalcDisplay` and implements a set of virtual calls it exposes, in particular `SetPrimaryDisplay`. https://github.com/microsoft/calculator/blob/2517854836fee634a273adf3be72371cdb51ddaa/src/CalcManager/Header%20Files/ICalcDisplay.h#L13 When setting a mode in `CalculatorManager`, e.g. https://github.com/microsoft/calculator/blob/2517854836fee634a273adf3be72371cdb51ddaa/src/CalcManager/CalculatorManager.cpp#L208 `this` (here: an instance of `CalculatorManager`) gets passed as an argument to the newly created `CCalcEngine` as `ICalcDisplay` pointer and the engine is stored as `unique_ptr` member field of `CalculatorManager`. In the destructor of `CalculatorManager`, a single function is called, `MemorizedNumberClearAll` which then invokes `ProcessCommand(IDC_MCLEAR)` on current engine, gets passed on to `CCalcEngine::ProcessCommandWorker`, to `DisplayNum`, to `CCalcEngine::SetPrimaryDisplay` and finally to `m_pCalcDisplay->SetPrimaryDisplay`, but here `m_pCalcDisplay` _was_ the instance of `CalculatorManager` that just got its destructor called. https://github.com/microsoft/calculator/blob/2517854836fee634a273adf3be72371cdb51ddaa/src/CalcManager/CalculatorManager.cpp#L46 https://github.com/microsoft/calculator/blob/2517854836fee634a273adf3be72371cdb51ddaa/src/CalcManager/CalculatorManager.cpp#L475 https://github.com/microsoft/calculator/blob/2517854836fee634a273adf3be72371cdb51ddaa/src/CalcManager/CEngine/scicomm.cpp#L87 https://github.com/microsoft/calculator/blob/2517854836fee634a273adf3be72371cdb51ddaa/src/CalcManager/CEngine/scicomm.cpp#L133 https://github.com/microsoft/calculator/blob/2517854836fee634a273adf3be72371cdb51ddaa/src/CalcManager/CEngine/scidisp.cpp#L124 https://github.com/microsoft/calculator/blob/2517854836fee634a273adf3be72371cdb51ddaa/src/CalcManager/CEngine/scicomm.cpp#L837 It will likely differ by implementation on how exactly, but the [standard suggests](http://eel.is/c++draft/class.cdtor#4) that will invoke the pure virtual `ICalcDisplay::SetPrimaryDisplay`. In case of GCC I believe the vtable is already destroyed by the time you enter dtor's body. There appears to be no reason to call `MemorizedNumberClearAll` in `CalculatorManager::~CalculatorManager()` because the calc manager and all its engines are going down anyway. Therefore, removing the call (and thus, the destructor which would then be empty). Fixes microsoft#563: Odd dependency cycle
* Fixed the WindowIdLog so that it is updated when a new WindowCreated event is fired * Updated the windowidlog check in LogWindowCreated to use IsWindowIdInLog
### Description of the changes: **1) Do not set units to default values if they already have valid values** This fixes the actual issue. `UnitConverter::InitializeSelectedUnits()` ( this function resets all units to their default units if available for the current category ) gets called after `UnitConverterViewModel::RestoreUserPreferences()` ( this function restores user preferences ). So Calculator has been restoring saved values, and then overriding the restored values with default values. I modified `InitializeSelectedUnits()` so that we only initialize units only when they are not already set to valid units for the current category. **2) Removed `m_isFirstTime`** I noticed that we are calling `RestoreUserPreferences()` twice when Calculator starts up, and the function is restoring the same value both times The below happens when Calculator starts up 1) On startup, in `UnitConverterViewModel::InitializeView()`, `RestoreUserPreferences()` is called. 2) `RestoreUserPreferences()` in turn triggers `OnUnitChanged()` 3) During the first call to `OnUnitChanged()`, m_IsFirstTime is `True`, so we call `RestoreUserPreferences()` again while also setting `m_IsFirstTime` to `False`. 4) `RestoreUserPreference()` again triggers `OnUnitChanged()` 5) During the second call to `OnUnitChanged()`, m_IsFirstTime is `False`, so we call `SaveUserPreferences()` I think we should only call `SaveUserPreferences()` inside `OnUnitChanged()` since we already restored user preferences during view initialization. I can't really think of a reason to restore units after view has been initialized. This led me to just delete `m_isFirstTime`. ### How changes were validated: Manually tested that units and the current category are properly selected when you quit and start Calculator.   ## Fixes microsoft#445.
…iling zeros if the number is integer (microsoft#256) * Currency rate: Compute how many decimals we need to display two meaningful digits at minimum * formatting * nit * Increase the number of meaningfull digits (2->4) * Revert "Increase the number of meaningfull digits (2->4)" This reverts commit 9ad93e0. * Rename constants * modify FORMATTER_RATE_* values * format CurrencyConverterUnitTests.cpp
…icrosoft#624) - Our documentation should make it clear to contributors what the expected developer environment is in order for the project to work. In this case, a user tried to run the UI tests, unaware that they needed WinAppDriver installed on their machine. This updates the README to indicate that requirement. - Trailing spaces from other parts of the document were also removed as part of this change Fixes microsoft#621
Commit 0722781 updated the app to use `DevAppName` for the app's window title when it was a non-official build, based on the state of `IsStoreBuild`. Unfortunately, `IsStoreBuild` is a _project_ level variable defined in [build-app-internal.yaml](https://github.com/microsoft/calculator/blob/0722781fc60565938da42ea252766b30d02e5fb5/build/pipelines/templates/build-app-internal.yaml#L36), but not a _compile-time_ defined value. To solve this, we are now defining `IS_STORE_BUILD` in `Calculator.vcxproj` when `IsStoreBuild='True'`, the same way that we set `SEND_DIAGNOSTICS` for official builds, and we'll change the window title based on that new `#define`. Using this new `#define` can lead us down a slippery slope. We need to limit the amount of divergent code that we have between dev/official builds. This should be hopefully one of very few instances where this value is ever used.
…osoft#632) Move back the VisualStateManager node to the root XAML element to fix visual states of the titlebar. ### How changes were validated: - Manually Fixes microsoft#631
* remove unused AlwaysOnTopExpressionItemTemplateSelector * replace incorrect ThemeResource by StaticResource references
* Make sure TitleBar takes into account the AOT mode change * remove namespaces in cpp files * code linting * use macro for IsAlwaysOnTop and make IsAlwaysOnTop/DisplayNormalAlwaysOnTopOption read-only * Fix FontWeight
…it lengths (microsoft#640) * Optimize BitFlipPanel * remove namespace in cpp file * improve localization + add tests * add helper to compare ivector * Modify how the control manages AutomationProperties::Name
…ities (microsoft#657) Aligning to recently published common SECURITY.md file available at https://github.com/microsoft/microsoft.github.io/blob/master/SECURITY.MD
sanderl
approved these changes
Sep 5, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pulled master into feature/GraphingCalculator branch:
-pulled master into feature/GraphingCalculator and resolved merge conflicts
How changes were validated: