search

Esri / coordinate-conversion-addin-dotnet

Addin for ArcMap and Pro for convenient coordinate conversion in Desktop.
Apache License 2.0
45 stars 20 forks source link

Implemented code for gitHub ticket #642 #644

Closed sumitzarkar closed 4 years ago

sumitzarkar commented 4 years ago

We have implemented code for following points:

  1. Removing: ArcMapAddinCoordinateConversion project/dll
  2. Removing:CoordinateConversionLibrary project/dll
  3. Refactoring the Pro project to remove dependencies on former CoordinateConversionLibrary a. remove view inheritance b. remove no-longer-needed classes (super/abstract classes) c. remove mediator/dispatcher (ex: convert to RelayCommands) d. use Pro styling for Dark Mode support
    1. As part of this if you notice any hard-coded UI strings that should be converted to Resources (for I18n) please convert these to resources
csmoore commented 4 years ago

I had a small build issue - not sure why it only showed up for me, but fixed in https://github.com/Esri/coordinate-conversion-addin-dotnet/pull/645

I also now see that the Units Tests don't run because of the new Pro dependencies - I'll try to figure out how to get these tests running for acceptance (like maybe just put these tests - CoordinateConversion\CoordinateConversionLibrary.Tests - in a quick addin just so we can run them)

csmoore commented 4 years ago

@sumitzarkar Sorry for my delay reviewing this PR - @OscarVilla1 tested successfully and I think the only issue I noticed was that there are 2 base classes that could be combined into one class/file:

  1. TabBaseViewModel and ProTabBaseViewModel
  2. OutputCoordinateViewModel and OutputCoordinateViewModel
csmoore commented 4 years ago

Opened alternate PR #646 - so we could just switch the head branch we are merging into (branch: Pro2.6 vs. dev). Feel free to move any discussion to that PR and sorry for any confusion.

csmoore commented 4 years ago

Closing unmerged to merge into different head branch in PR #646