Esri / distance-direction-addin-dotnet

Add-in provides the ability to easily and quickly create geodesy lines, circles, ellipses and range rings.
Apache License 2.0
17 stars 22 forks source link

Add out of bounds checks for click points in Pro #650

Closed csmoore closed 5 years ago

csmoore commented 5 years ago

See #634

ArcMap already had a similar check, just added the same check/test to Pro.

dfoll commented 5 years ago

"Before"

with MT built 212 image

'After'

with DD testing build 27 image

csmoore commented 5 years ago

We can change the behavior - I think the only reason it was done this way in Pro was:

  1. It was done this way in ArcMap
  2. There is a performance hit to check that the point is within the extent of all the map layers so doing this check on every mouse may slow things down - for the circles another approach is used (the circle create just fails in Pro so we get an immediate indication)
dfoll commented 5 years ago

I can call in a little bit to discuss. I am proposing doing it the way it is done in ArcMap (no message is displayed until mouse click). This is the way it was recently done for lines in Pro, but not for Circles/Ellipses/Rings ... wondering which is our best approach

in either case I don't think we must match ArcMap, that is ust a good example (that is not in flux) of the behavior I personally like. The most important part is that all four within Pro match. I have a minor use-case based reason why I like the "ArcMap way" of doing things better that I can show when we talk

csmoore commented 5 years ago

I think the circles/rings were done differently because there was an issue where we couldn't even create the geodesic feedback circles outside the extent, though I can't seem to find this particular issue (maybe @kgonzago remembers), though I can find plenty of others about the clicking outside the extent, https://github.com/Esri/distance-direction-addin-dotnet/issues/512 , https://github.com/Esri/distance-direction-addin-dotnet/issues/535 https://github.com/Esri/distance-direction-addin-dotnet/issues/547

dfoll commented 5 years ago

discussed this with @lfunkhouser. Our approach with this is to move forward with merging this PR. We are going ahead with the understanding that Lines "out of bounds behavior" is slightly different that Circle/Ellipse/Ring "out of bounds behavior". The difference in behavior is because previews for circular items draw/behave different than line previews. There is a limitation with drawing circular previews outside of the extent. To make them all match we would need to implement something to check in/out of extent for each mouse move for lines. We have decided the performance hit to do that is not currently worth the return value.