Esri / visibility-addin-dotnet

ArcGIS Add-in provides the capability to quickly do line of sight analyses.
Apache License 2.0
14 stars 19 forks source link

Separate ArcMap and ArcGIS Pro code #341 #343

Closed csmoore closed 4 years ago

csmoore commented 4 years ago

We are just switching the head branch we are merging PR #685 into (branch: Pro2.6 vs. dev).

Implements: #341 Original PR: #342 (we will close that PR unmerged after this PR is merged)

Original PR #341 comments from @sumitzarkar :

Implemented following points:

  1. ArcMapAddinVisibility project/dll

  2. VisibilityLibrary project/dll

  3. Refactoring the Pro project to remove dependencies on former VisibilityLibrary 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. Localisation

  5. Make necessary changes by removing CoordinateConversionLibrary and replacing with ProCoordinateConversion as done in DistanceAndDirection AddIn.

csmoore commented 4 years ago

@sumitzarkar We have finished our review and testing, the only remaining issues to close and merge this PR are:

  1. Dark Mode issue: https://github.com/Esri/visibility-addin-dotnet/issues/346
  2. Button Centering Issue: https://github.com/Esri/visibility-addin-dotnet/issues/345

I haven't had a chance to review the code, but I will do this today and let you know if I notice any additional code-related issues - but this project is simpler so I don't notice anything obvious on first look.

sumitzarkar commented 4 years ago

@lfunkhouser @csmoore @dfoll

We have fixed above mentioned two issues (commit link ).

Implemented following github tickets:

csmoore commented 4 years ago

Issue #345 and #346 have been tested and closed - merging this PR.

Note: we realize the the issues that @jmccausland found in D&D and CC (ex. NotificationObject, etc.) also should be resolved prior to moving this code.