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

Separate ArcMap and ArcGIS Pro code #642

Closed lfunkhouser closed 4 years ago

lfunkhouser commented 4 years ago

To prepare for converting the addin to a Pro-only component going forward, we are making some structural changes to the solution:

  1. Removing: ArcMapAddinCoordinateConversion project/dll
  2. Removing: CoordinateConversion project/dll
  3. Refactoring the Pro project to remove dependencies on former CoordinateConversion 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
  4. 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
sumitzarkar commented 4 years ago

@dfoll @lfunkhouser @csmoore @OscarVilla1

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

Note: This build is work in progress and some build errors can be observed

sumitzarkar commented 4 years ago

@dfoll @lfunkhouser @csmoore @OscarVilla1

We have completed refactoring, and have committed the code for it.

Note: We have observed that addin is crashing in one case and we are working on it.

sumitzarkar commented 4 years ago

@lfunkhouser @csmoore @OscarVilla1@dfoll

Below are the repo steps to reproduce the crash we were experiencing:

  1. Launch the AddIn
  2. Activate “Map Point Tool”
  3. Click on “Add” button under “Output” section
  4. ArcGIS pro crashes with an error message

We have found the root cause for this and have fixed at our end, and we are testing it.

sumitzarkar commented 4 years ago

@lfunkhouser @csmoore @OscarVilla1 @dfoll

We have pushed the code to fix the above mentioned crash.

sumitzarkar commented 4 years ago

@lfunkhouser @csmoore @OscarVilla1 @dfoll

We have pushed the code Pro Style changes.

csmoore commented 4 years ago

@sumitzarkar - would it be possible to change the buttons also - like you did for DistanceAndDirection - thanks

sumitzarkar commented 4 years ago

@csmoore @dfoll @lfunkhouser

We have submitted the pull request for this ticket Pull Request Link

lfunkhouser commented 4 years ago

@csmoore will merge PR into a "Pro 2.6 branch"

sumitzarkar commented 4 years ago

@lfunkhouser @csmoore @dfoll

We have implemented the following code review comment and here is the commit link:

  1. Instead of NotificationObject, use the Pro sdk provided ViewModelBase
    1. Add the dependencies in Config.daml
sumitzarkar commented 4 years ago

@lfunkhouser @csmoore @dfoll

We implemented code to fix pro style issue for browse window commit link:

csmoore commented 4 years ago

✔️ These issues have been tested and closed (Dark Mode Form issues) : #647 , #648

The remaining issues are :

  1. Combined duplicate Base Classes into one class/file: TabBaseViewModel and ProTabBaseViewModel OutputCoordinateViewModel and ProOutputCoordinateViewModel
  2. Change a few base class objects from ViewModelBase to PropertyChangedBase - comment here: https://github.com/Esri/coordinate-conversion-addin-dotnet/pull/646/commits/eff8118d3ece199b7831cf2ff861ef7427fec30c#r389223919

(This comment also included on PR)

sumitzarkar commented 4 years ago

@lfunkhouser @csmoore @dfoll

We implemented the code to combine duplicate Base Classes into one class/file (commit link):

  1. TabBaseViewModel and ProTabBaseViewMode
  2. OutputCoordinateViewModel and ProOutputCoordinateViewModel
BrandonArmstrong commented 4 years ago

Verified following fixes while running through Coordinate Conversion add-in Checklist located here https://github.com/ArcGIS/solutions-defense-test-catalog/issues/279

Verification was done using: ArcGIS Pro v.2.6.0 alpha build 23705 Coordinate Conversion add-in version attached to issue CoordinateConversion.zip