Hello C# - Migrating the codebase from C++/CX to C# (Phase 1) #1598

tian-lt · 2021-07-01T07:04:51Z

Hello C# - Migrating the codebase from C++/CX to C# (Phase 1)

This PR is Related to issue #893

Description of the changes:

Migrates the View layer from C++/CX to C#
Changes CalcViewModel into a Windows Runtime Component library
Adjusts pipelines and project configuration for the C# codebase

How changes were validated:

Manual Tests Results for Branch feature/UICSharpCalculator

Test State: Completed
Test Health: Green
Platforms(s): Desktop VM & Desktop & Surface Pro 7+ & HP laptop
OS Build(s): RS4 (17134.1.180410-1804), RS5 (17763.1.180914-1434), 21H1(19043.1052), VB (19041.1.191206-1406), Co (22000.1)
App Build: Calculator -Release 0.2106.1402.0

* change CalcViewModel into a WindowsRuntimeComponent project (#5) * change CalcViewModel into a WindowsRuntimeComponent project * remove the old UI codebase (#6) * initially migrated C# codebase by tian (#7) * initial migrated C# codebase by tian * format the codebase * resolve comments * undo: modifications on UI test project * Remove the blocks that have more than 1 empty line. * Register DP using keyword 'nameof' * C# Migration: Initially migrated C# codebase by Han (#8) * C# Migration: Initially migrated C# codebase by Han * Resolved comments and misssing asset * Added three files to Calculator project * Added TODO comment and updated Object * NavCategory: temporary resolution of the hang issue (#9) * Updated CalcViewModel and missing files (#10) * Updated CalcViewModel and WinMeta * Added Calculator.rc * Resolved comment for InitializeLocalizationSettings * add: views/unitconverter.xaml (#11) * add: views/unitconverter.xaml * format the code * remove the extra empty line * add an empty line * check null before invoking event handlers (#12) * fix problems of the migration of OBSERVABLE_PROPERTY_RW (#13) * fixes crash in MathRichEditBox.ctor() (#14) * fixes crash in MathRichEditBox.ctor() * typo * Update azure-pipelines.ci.yaml for Azure Pipelines * Added a link copy of CalcViewModel to temporarily pass Unit Tests (#16) * Updated CalcViewModelCopyForUT configuration (#17) * changes output path of the UI project to align with other projects (#15) * fixes EETypeLoadException issue: export class DelegateCommand (#18) * fixes EETypeLoadException issue: export class DelegateCommand * weak-reference in C++/CX * WeakRef in C# codebase * UTF-8-BOM * spaces in macro * resolve some comments from the offline review * format * rename file * fixes the memory list issue (#20) * fixes a wrongly migrated property * UTF-8-BOM * fixes up the crash of type casting (#21) * Update localized strings 2021-01-04 (microsoft#1458) (#23) (cherry picked from commit cdcb956) Co-authored-by: Matt Cooley <macool@microsoft.com> * Fixup tests (#1429) (#24) - Removed unneeded "ToString" calls - Fixed typos - Renamed "fEButtonState" to "FEButtonState" (cherry picked from commit 66ad328) Co-authored-by: N <71219152+PokeCodec@users.noreply.github.com> * Update graph internal engine verseion (microsoft#1466) (#25) (cherry picked from commit 0048dcb) Co-authored-by: Quentin Al-Timimi <27322516+quentin987@users.noreply.github.com> * Turn off DFS file shares in internal build system (microsoft#1470) (#26) (cherry picked from commit 885fa23) Co-authored-by: Matt Cooley <macool@microsoft.com> * Improve clarity of math expressions in history for Standard Calculator (feature microsoft#138) (microsoft#1453) (#27) * Implemented feature & added unit tests * Fixed more unit/ui tests * Refactored tests * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp * Update HistoryTests.cpp (cherry picked from commit 565e3e2) Co-authored-by: Wei (Waley) Zhang <waley.zhang@microsoft.com> * Adds unit-test cases for NarratorAnnouncement after fixing issue microsoft#1386 (microsoft#1469) (#28) * fix bug: No confirmation is announced by the narrator after activating 'Remove equation' button microsoft#1386 * Unit Test: Add NarratorAnnouncementUnitTests Co-authored-by: tain <tankle_@hotmail.com> (cherry picked from commit 9d8e2ad) Co-authored-by: Tian L <60599517+MSFT-Tilia@users.noreply.github.com> * Move localization pipeline sync schedule to the YAML file (microsoft#1478) (#30) (cherry picked from commit 007eccd) Co-authored-by: Matt Cooley <macool@microsoft.com> * remove the strong reference carried from delegate (#32) * Remove the finalizer of ControlSizeTrigger (#31) * Normalize the namespace of CalcViewModel (#33) * ViewMode: arrange namespaces * UI build pass * run release * UT build pass * pass build * resolve comment: make the diff results cleaner * resolve comment: make the diff results cleaner (2) * resolve comment: make the diff results cleaner (3) * resolve comment: move impl into a namespace * update: spaces * update: CalculatorButtonUser.h * UTF-8 to UTF-8-BOM * remove ViewState.h/.cpp from CalcViewModel path * revert changes for NavCategory.cpp * remove extra space * remove UCM * remove BOM * Fixed a graphing calculator "permissions" bug caused by PR microsoft#1426 (microsoft#1471) (#34) - The PR microsoft#1426 can cause a crash when no users are returned via `User::FindAllAsync(UserType::LocalUser)` when subsequently trying to access the first user. The existing code also does not guarantee that the returned user is the currently active user. - This fix retrieves the user that opened the app and passes this user into a function to check if this user has the proper permissions to access the graphing mode. This makes sense since the active user is indistinguishable (at least from the app's perspective) to the user who opened the app. This user's permissions are then propagated downwards to properly set up the navigation menu of the app. - Implementation detail worth pointing out: `s_categoryManifest` is what is used to populate the navigation menu of the app, but this variable is static by design, so a separate function was written to override the appropriate `isEnabled` value in `s_categoryManifest`. This function is called by `onLaunched`. - Manual testing Co-authored-by: Wei (Waley) Zhang <waley.zhang@microsoft.com> * fixes up a bug (#35) * fix csproj (#37) Co-authored-by: hanzhang54 <zhangh@microsoft.com> Co-authored-by: Matt Cooley <macool@microsoft.com> Co-authored-by: N <71219152+PokeCodec@users.noreply.github.com> Co-authored-by: Quentin Al-Timimi <27322516+quentin987@users.noreply.github.com> Co-authored-by: Wei (Waley) Zhang <waley.zhang@microsoft.com> Co-authored-by: Tian L <60599517+MSFT-Tilia@users.noreply.github.com>

* fix

…soft#1550) * **BYPASS_SECRET_SCANNING** * fix * fixes x86/ARM/ARM64 builds for CI-Pipeline

…oft#1551) Things that required such update included: * `wstringstream` * `setprecision` * `SCODE_CODE`, `E_BOUNDS` * Various SAL macros Co-authored-by: Michał Janiszewski <janisozaur@users.noreply.github.com>

Co-authored-by: Matt Cooley <macool@microsoft.com>

* #DEBUG is a known C# preprocessor directive * So far, we haven't observed the problem described in the comment from C# async * fixes misc TODO items

* Add CI-internal pipeline * No ARM64, to match release Co-authored-by: Matt Cooley <macool@microsoft.com>

Co-authored-by: Matt Cooley <macool@microsoft.com>

* remove WinMeta.cs * undo a trivial change * UTF-8 BOM

* fixes: GraphingNumber doesn't work correctly * Avoid crashing

…icrosoft#1595) Co-authored-by: Dave Grochocki <grochocki@users.noreply.github.com>

… Mode (microsoft#1596) * keep the value away from getting rounded * set the display precision to 6 to align with C++ impl

…to merge/tomaster

mcooley · 2021-07-09T01:58:36Z

src/build/Calculator.StampAssemblyInfo.targets

+    generated file.
+  -->
+
+  <PropertyGroup Condition="'$(Version)' != ''">


You'll probably want to use $(AppVersion) instead of $(Version) in this file, since AppVersion is what's being passed on the command line here.

Another option would be to change build-single-architecture.yaml to use Version instead of AppVersion. If you do that you'll need to change other projects in the repo which currently use AppVersion.

Thanks Matt,
I decided to use Version instead of AppVersion since build-single-architecture.yaml is the only producer who specifies AppVersion. Another reason for doing such way is that I referred to Clock project and I suppose it'd be better to minimize the differences of engineering practice between EE projects.
Please see commit 43634b9 for the changes.

Also, edited AssemblyInfo.cs to provide the information that was declared in Calculator.rc. The changes are in commit
4ed33a7.

After CI-pipeline finished successfully, I've verified the properties of file CalculatorApp.exe and it has the expected version information.

Sounds good--just make sure that the .dll files in the package still have version information just like CalculatorApp.exe.

Double checked the master branch, only Calculator project(the UI project) has assembly-info. The other projects, such as CalcViewModel, GraphControl, GraphingImpl, TraceLogging, they are tagging with default assembly-info which is empty. CalcViewModel was a static-library, so it's fine if it doesn't have assembly-info, but the remaining projects don't look good.

I've assigned some assembly-info for all the shared library(.dll) projects in commit 8926e99. Let me know if it's not good to do so and I'll revert the commit.

BTW, if we want to add assembly-info for GraphingImpl.dll, we need to submit some changes in the internal repo. But that task should not block this PR from getting checked in, I suppose.

Hi @mcooley , this PR has been paused for a while. We'd like to make it checked in first. If there's any problems, we can send out other PRs to fix them.

…into merge/tomaster

hanzhang54

Thank you Matt and Tian for reviewing and updating. This looks very good to me!

hanzhang54

LGTM

xuhongxu96 · 2021-07-15T07:01:59Z

DesignData has not been converted to C#?

src/Calculator/App.xaml.cs

src/Calculator/Views/StateTriggers/CalculatorProgrammerDisplayPanel.xaml

tian-lt · 2021-07-15T09:11:43Z

DesignData has not been converted to C#?

The DesignData files are not being included as translation units. Keeping it as what it is doesn't block the progress of C# migration.

tian-lt and others added 30 commits May 25, 2021 17:17

**BYPASS_SECRET_SCANNING** (microsoft#1546)

fd77a00

Fixes a bug about the UI of expression tokens (microsoft#1547)

3c69bf1

* fix

[FeatureBranch] Fixes x86/ARM/ARM64 builds for the CI-Pipeline (micro…

ce4ced5

…soft#1550) * **BYPASS_SECRET_SCANNING** * fix * fixes x86/ARM/ARM64 builds for CI-Pipeline

Add headers missing for compilation with GCC (microsoft#1468) (micros…

2d166ed

…oft#1551) Things that required such update included: * `wstringstream` * `setprecision` * `SCODE_CODE`, `E_BOUNDS` * Various SAL macros Co-authored-by: Michał Janiszewski <janisozaur@users.noreply.github.com>

Update nuget.config file (microsoft#1486) (microsoft#1552)

d224303

Co-authored-by: Matt Cooley <macool@microsoft.com>

Fixes up some simple miscellaneous TODO items (microsoft#1556)

ce5a1cb

* #DEBUG is a known C# preprocessor directive * So far, we haven't observed the problem described in the comment from C# async * fixes misc TODO items

resolve some warnings (microsoft#1564)

1fa8c4b

Add internal CI pipeline (microsoft#1553) (microsoft#1565)

e06be0c

* Add CI-internal pipeline * No ARM64, to match release Co-authored-by: Matt Cooley <macool@microsoft.com>

Temporarily disable PGO NuGet package (microsoft#1510) (microsoft#1566)

e9890d1

Co-authored-by: Matt Cooley <macool@microsoft.com>

[C# Calc]Removes WinMeta.cs (microsoft#1567)

12a79d5

* remove WinMeta.cs * undo a trivial change * UTF-8 BOM

[C# Calc] Reverts some changes for Currency constants (microsoft#1570)

1d345ee

Update2108release - experimental (microsoft#1572)

b31879f

adjusts Calculator.csproj (microsoft#1571)

e510645

fixes BinSkim problems (microsoft#1573)

3ab348a

fixes an issue around line style (microsoft#1575)

6196c89

fixes the missed NULLs (microsoft#1576) (microsoft#1578)

88715b4

Fix the Missing Part in Unit Converter Constructor (microsoft#1579)

9017f07

fixes: calculator doesn't remember its previous mode (microsoft#1580)

ae5d6c6

Fixes: GraphingNumber doesn't work correctly (microsoft#1585)

cc8f8b4

* fixes: GraphingNumber doesn't work correctly * Avoid crashing

fixes binding (microsoft#1586)

40ad24f

resolve TODO items (microsoft#1589)

bd52851

Improving keyboard support in VariableValueSlider (microsoft#1559) (m…

74ef763

…icrosoft#1595) Co-authored-by: Dave Grochocki <grochocki@users.noreply.github.com>

[C# Calc] Fixes: Keep the value away from getting rounded in Graphing…

4ae898d

… Mode (microsoft#1596) * keep the value away from getting rounded * set the display precision to 6 to align with C++ impl

fixes the button-light-up time (microsoft#1597)

d6ed2cb

Merge remote-tracking branch 'upstream/feature/UICSharpCalculator' in…

9411711

…to merge/tomaster

fixes up merging flaws

04894a0

Update2108release

5f36572

fixes (microsoft#1599)

cc9a46a

Merge remote-tracking branch 'upstream/feature/UICSharpCalculator' in…

a54cb96

…to merge/tomaster

tian-lt added 5 commits July 7, 2021 12:16

remove the Resources filter from CalcViewModel project

b3875f0

removes that since this can be captured automatically

27f8771

AppxBundlePlatforms

735b81b

StampAssemblyInfo

8915ce1

removes PreferredToolArchitecture

05e31bc

tian-lt requested review from mcooley and volkcor July 7, 2021 10:55

grochocki added Enhancement and removed Enhancement labels Jul 8, 2021

mcooley reviewed Jul 9, 2021

View reviewed changes

tian-lt added 3 commits July 9, 2021 16:29

Change the arg AppVersion into Version

c84df57

Change the arg AppVersion into Version

43634b9

Merge branch 'merge/tomaster' of https://github.com/tian-lt/calculator …

5dc1e42

…into merge/tomaster

hanzhang54 previously approved these changes Jul 9, 2021

View reviewed changes

from Calculator.rc to AssemblyInfo.cs

4ed33a7

tian-lt dismissed hanzhang54’s stale review via 4ed33a7 July 9, 2021 09:08

tian-lt requested review from hanzhang54 and mcooley July 9, 2021 09:42

Adds assembly-info

8926e99

hanzhang54 approved these changes Jul 15, 2021

View reviewed changes

xuhongxu96 suggested changes Jul 15, 2021

View reviewed changes

src/Calculator/App.xaml.cs Show resolved Hide resolved

src/Calculator/Views/StateTriggers/CalculatorProgrammerDisplayPanel.xaml Show resolved Hide resolved

ghost added needs author feedback and removed needs author feedback labels Jul 15, 2021

tian-lt requested a review from xuhongxu96 July 15, 2021 09:09

tian-lt merged commit fdae100 into microsoft:master Jul 15, 2021

tian-lt deleted the merge/tomaster branch July 15, 2021 09:12

hanzhang54 mentioned this pull request Aug 9, 2021

Calculator on Graphing mode crashes when I click on either "share" button or "switch to equation mode" button. #1592

Closed

tonyqus mentioned this pull request May 17, 2022

Migrate codebase to C# #893

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hello C# - Migrating the codebase from C++/CX to C# (Phase 1) #1598

Hello C# - Migrating the codebase from C++/CX to C# (Phase 1) #1598

tian-lt commented Jul 1, 2021

mcooley Jul 9, 2021

tian-lt Jul 9, 2021 •

edited

Loading

mcooley Jul 9, 2021

tian-lt Jul 11, 2021

tian-lt Jul 15, 2021

hanzhang54 left a comment

hanzhang54 left a comment

xuhongxu96 commented Jul 15, 2021 •

edited

Loading

tian-lt commented Jul 15, 2021

Hello C# - Migrating the codebase from C++/CX to C# (Phase 1) #1598

Hello C# - Migrating the codebase from C++/CX to C# (Phase 1) #1598

Conversation

tian-lt commented Jul 1, 2021