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

Map Point tool issues (ArcMap) #293

Closed elinz closed 7 years ago

elinz commented 7 years ago

@nfeuerstein My apologies. I thought I had submitted this issue last night when I had submitted the same issue for the ArcGIS Pro addin.

Tested build 18 Jan 2017 12:25 (#1140).

When the Map Point tool is disabled, the cursor icon does not change back to the cursor icon that was set before the Map Point tool was enabled.

FYI: All other behavior is what I would expect:

nfeuerstein commented 7 years ago

@elinz can you test this again? I believe @dhagens fixed this with a pull request this evening.

elinz commented 7 years ago

Yep. I will login and test right now.

-------- Original message -------- From: Natalie Feuerstein Date:01/18/2017 9:14 PM (GMT-08:00) To: Esri/coordinate-conversion-addin-dotnet Cc: Eric Linz , Mention Subject: Re: [Esri/coordinate-conversion-addin-dotnet] Map Point tool issues (ArcMap) (#293)

Assigned #293https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Esri_coordinate-2Dconversion-2Daddin-2Ddotnet_issues_293&d=DwMCaQ&c=n6-cguzQvX_tUIrZOS_4Og&r=2ZpT4vzBgD3Uq0Qu1_1lvg&m=ndTE_OkTFqWeEBQG_-6v0BFs46vGbf8gVy5b6wLJ_VQ&s=VzXpmhusS0cLY5RDHPIpHo6KOAc-PQm4bULeFBMbQwk&e= to @elinzhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_elinz&d=DwMCaQ&c=n6-cguzQvX_tUIrZOS_4Og&r=2ZpT4vzBgD3Uq0Qu1_1lvg&m=ndTE_OkTFqWeEBQG_-6v0BFs46vGbf8gVy5b6wLJ_VQ&s=x84HHqvwzSz7DJjWu2yIDNsE1HGkhQJqZ4shG_Bkgtw&e=.

- You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Esri_coordinate-2Dconversion-2Daddin-2Ddotnet_issues_293-23event-2D928579175&d=DwMCaQ&c=n6-cguzQvX_tUIrZOS_4Og&r=2ZpT4vzBgD3Uq0Qu1_1lvg&m=ndTE_OkTFqWeEBQG_-6v0BFs46vGbf8gVy5b6wLJ_VQ&s=NnbslJPedxWilbpMZPMsIwlF8ZV7K_t0RNqjEfKDeSw&e=, or mute the threadhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_AEa7Yl0cpNwEpah4KQnVvzeBMy4jgQf6ks5rTvEXgaJpZM4Lnm9I&d=DwMCaQ&c=n6-cguzQvX_tUIrZOS_4Og&r=2ZpT4vzBgD3Uq0Qu1_1lvg&m=ndTE_OkTFqWeEBQG_-6v0BFs46vGbf8gVy5b6wLJ_VQ&s=rEFHAl9dxOzQ5RNGtmm8iEUuSxbAKWY-zlTUbFF2fW0&e=.

elinz commented 7 years ago

@nfeuerstein Coordinate Conversion still crashes in latest build, but the Map Point tool issue in Distance Direction was fixed (https://github.com/Esri/distance-direction-addin-dotnet/issues/291), so it looks promising that the Map Point tool issue in Coordinate Conversion was fixed and I can verify when the crash issue #292 is fixed.

dhagens commented 7 years ago

@elinz, this is ready for testing

nfeuerstein commented 7 years ago

@elinz has this been tested?

elinz commented 7 years ago

@nfeuerstein Tested previous build and it wasn't working. Maybe it was timing of build with the merge. Now that another build is available I'm testing it right now.

elinz commented 7 years ago

@dhagens @nfeuerstein @kgonzago Tested in the last two builds which were after the merge and this fix isn't working. In fact there are more issues now.

When I submitted this issue, the Map point tool on CC was disabling; the only thing it needed was the cursor to change from the crosshairs to whatever it was before the Map point tool was enabled; however, I can't even disable the Map point tool now.

Assigning back to @dhagens

dhagens commented 7 years ago

I'll have to test with the toolbar because everything is working fine at the component level

dhagens commented 7 years ago

@elinz, I verified that the current build is not working as it should in the Toolbar. Could you test the tool at the component level to see if it is working correctly as I am seeing?

elinz commented 7 years ago

@dhagens This may be a problem. I'm trying to run the Jenkins build to build the CC at the component level but it's failing.

nfeuerstein commented 7 years ago

verified with Hagens that its working at the component level somethings happening at the single add-in and/or with Jenkins thats causing it to not behave the same Kevin is going to look into it in a bit and Hagens will reach out the Kevin in case he needs anything

dhagens commented 7 years ago

@kgonzago, I have a PR to review and merge. I moved the IsToolActive property to the ArcMapTabBaseViewModel and removed it from the ConvertViewModel. I don't know if this might be causing issues in the single addin or not. Kick off a new build after the merge and let me know if it changes anything. Everything still seems to work properly at the component level.

kgonzago commented 7 years ago

@dhagens PR #299 was merged. I also confirmed your latest change in the single add-in works. Map tool now shows with a cross-hair cursor and disables appropriately.

Once build is done, @elinz can you confirm?

elinz commented 7 years ago

@dhagens @nfeuerstein @kgonzago I'll start a Jenkins build now.

elinz commented 7 years ago

@dhagens @nfeuerstein @kgonzago Tested the single addin build 19 Jan 2017 19:11 Build Number 1159 and the Map Point tool on coordinate conversion still can't be disabled.

nfeuerstein commented 7 years ago

@ACueva could this be a Jenkins issue?

ACueva commented 7 years ago

@nfeuerstein not likely. Jenkins hasn't changed the build process for the single addin since @kgonzago and I implemented it. I'll log in a few and double check directly from the Jenkins build location to make sure we are both pulling from the correct repo, and correct branches.

ACueva commented 7 years ago

Just checked. Jenkins appears to be pulling from the correct repositories and branches.

nfeuerstein commented 7 years ago

@kgonzago @dhagens can you take another look? @elinz is still seeing an issue with the map point tool in a build he kicked off after the merge last night.

kgonzago commented 7 years ago

@nfeuerstein @elinz Good news. Issue fixed. @dhagens will fold in changes in the next build.

elinz commented 7 years ago

@dhagens @kgonzago @nfeuerstein Great. I will start another Jenkins build.

dhagens commented 7 years ago

I already started one, 1164 should be done in a few minutes

dhagens commented 7 years ago

@elinz @nfeuerstein @kgonzago , I tested the addin in build 1166 and all is working. Can you close this issue before the War room meeting? The build failed on documentation but the addin was created successfully.

elinz commented 7 years ago

@dhagens @nfeuerstein @kgonzago I'm going to grab the addin from the dated build folder and not from "current" and test that. I will do that right now.

elinz commented 7 years ago

@dhagens @nfeuerstein @kgonzago Verified fixed on build 20 Jan 2017 number 1167. Closing issue. Thanks for getting this fixed.

nfeuerstein commented 7 years ago

YEEEESSSS!!!