search

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

Distance and Direction - Rings - Allow user to create unequally spaced rings through keyboard #445

Closed csmoore closed 6 years ago

csmoore commented 7 years ago

From @elinz on January 26, 2017 17:59

Widget

Distance and Direction

Version of widget

Bug or Enhancement

Repo Steps or Enhancement details

We allow the user to interactively create unequally spaces rings... image

It would be very useful to be able to create unequally spaces rings through the keyboard. For user input the user could enter a delimited list of distances, for example... image

Copied from original issue: Esri/solutions-webappbuilder-widgets#822

csmoore commented 7 years ago

From @adgiles on February 1, 2017 16:56

@elinz

Updated please verify that it behaves as expected

csmoore commented 7 years ago

From @lfunkhouser on May 2, 2017 4:52

Assigning to MT PEs to verify. @dfoll @topowright @ACueva @NatalieCampos

csmoore commented 7 years ago

From @ACueva on May 2, 2017 21:6

I got this one.

csmoore commented 7 years ago

From @ACueva on May 2, 2017 23:12

Tested and this sort of works. I say this because this works:

2017-05-02_16-10-11

If we add spaces to the entry, it no longer works:

2017-05-02_16-11-17

IMO the second would be what a user would enter. @nfeuerstein @lfunkhouser thoughts?

csmoore commented 7 years ago

From @ACueva on May 2, 2017 23:13

@kgonzago

csmoore commented 7 years ago

From @topowright on May 2, 2017 23:22

@ACueva had an issue with this tool not calculating properly. Here is an example of what is happening:

image

The outer circle should be 6000 meters, but it is 3000 meters, which means that the last circle is calculating from the center point and not the second to last ring.

csmoore commented 7 years ago

From @topowright on May 2, 2017 23:26

Here is another example of this tool calculating it incorrectly:

image

csmoore commented 7 years ago

From @topowright on May 2, 2017 23:29

This tool would work if we changed the name of the tool from distance between rings to distance from origin. This is per @ACueva logic of what this tool could be vs. looking at what it is not.

If we choose to do this we should still log an enhancement from Distance Between Rings.

csmoore commented 7 years ago

From @topowright on May 2, 2017 23:30

@kgonzago @nfeuerstein @lfunkhouser

csmoore commented 7 years ago

From @lfunkhouser on May 3, 2017 0:0

Dev work is required either way.

Including an option to draw rings based on distance from origin would require a UI enhancement. This would give the user an option to select either draw rings based on distance between rings (as currently designed) or draw based on distance from origin (how it is currently behaving).

If we plan to include this enhancement, it is not working as expected and would require dev work.

My recommendation is to revert this change and add to planned work for a future release.

FYI @nfeuerstein

csmoore commented 7 years ago

From @kgonzago on May 4, 2017 15:9

@lfunkhouser I would suggest we stick with what's implemented and change the label to "Distance from Origin". We then tackle "Distance between Rings" functionality in the next iteration. Thoughts?

csmoore commented 7 years ago

From @topowright on May 4, 2017 19:21

changing the label is not possible because it will affect the other workflow that is already in place.

csmoore commented 7 years ago

From @ACueva on May 10, 2017 15:24

@nfeuerstein @lfunkhouser If this item is to be addressed this version, PE's will need to sequence diagram and plan for these changes. Please confirm these changes should be incorporated into this release.

csmoore commented 7 years ago

From @adgiles on May 12, 2017 13:16

Here is my suggestion, the current widget UI looks something like:

1

I recommend adding a drop down with the following 4 creation types (Interactively | Fixed | Origin | Cumulative). Then change the UI depending on the choice:

2 3 4 5

For Origin and Cumulative the UI element is like a table that allows distances to be added or removed similar concept as the Multl Ring Buffer in desktop

topowright-zz commented 6 years ago

@dfoll will take a closer look at this.

dfoll commented 6 years ago

@adgiles i'm looking at your UI design while sitting in the meeting right now. I like the design. I'd say go with it, and i'd like to test it. I have one question that i would bring up, and maybe you have thought this through already....when doing "from origin" (which is done through the keyboard) what happens if the value in row 2 is a smaller value than row 1? would the tool actually be drawing the values in the order they are in the list, or is that list basically just a collection of values.... another way of asking is does row 1 =ring 1 and row 2 = ring 2 etc.? .... just thinking forward, wondering if there is a problem if the value in row 2 is smaller than row 1 .... if there is rooms for my input in this, i would say row1 does not need to equal ring 1, that the "table" is just a list of values

adgiles commented 6 years ago

@dfoll it would just be a list of values, and loop through them so the order wouldn't really matter

dfoll commented 6 years ago

@adgiles cool, sounds good, that's what i figured, just trying to think of what i could, before i have one to physically use, but as far as what you have for your UI design, i like the way it looks and the way i believe it is intended to function

adgiles commented 6 years ago

@dfoll

Development work complete in this PR #486:

capture

capture

capture

untitled

Should be good to test once merged

dfoll commented 6 years ago

@adgiles cool, i'll work with this today, should have something for you when you are back in tomorrow

dfoll commented 6 years ago

I am still testing this, looks good so far. tomorrow morning will be more testing and documenting.

topowright-zz commented 6 years ago

We are not going to be able to finish the documentation on this item before the end of the sprint. We need to pull this out of the sprint. @lfunkhouser

topowright-zz commented 6 years ago

We need to understand what documentation needs to be finished.

topowright-zz commented 6 years ago

This needs to be documented for WAB 6.1

BobBooth commented 6 years ago

The dev daily build snapshot of web app builder 2.8: arcgis-web-appbuilder-2.8-snapshot.zip is broken. Went over this with @kgonzago numerous dependencies not present, including: dbutilities.js db-engine.js

NPM installs missing: cookie parser body parser
express

JSON: config.json log4json.json

Ran out of time trying to get build working.

BobBooth commented 6 years ago

A new snapshot file, dated today, is now available. I'm grabbing it and giving that a try. @kgonzago

BobBooth commented 6 years ago

This WAB version from today is working.

BobBooth commented 6 years ago

@adgiles @kgonzago
Testing the Origin and Cumulative construction methods of creating Rings, found a few problems. 1) When I'm adding values to this list, I click in the "Double click to add value" box, and the text is not selected, so I have to manually select and delete it before entering a value. 2) There seems to be no way to delete distance values entered in the Origin/Cumulative methods of creating rings. Deleting the text gets rid of it temporarily, but pressing Enter or clicking in another box then restores the value. image.png Clear Graphics, switching to another tab, and switching to another method all also fail to reset the values. 3) After entering a value, pressing Enter does not commit the value and move on to the next box (or next item in the GUI).

tested in daily build (2/20/18 of Web App Builder DE and the latest widget downloaded as a zip from the arcgis-webappbuilder-kevi2741-distance-and-direction branch of the repo.

BobBooth commented 6 years ago

Aside from that, the new rings functionality seems to be working correctly.

BobBooth commented 6 years ago

Submitted issues: https://github.com/Esri/distance-direction-addin-dotnet/issues/508 https://github.com/Esri/distance-direction-addin-dotnet/issues/507 https://github.com/Esri/distance-direction-addin-dotnet/issues/506

BobBooth commented 6 years ago

Seems like some controls from the earlier design are missing: image.png vs image.png

BobBooth commented 6 years ago

After testing in WAB(DE)2.8, I went back and grabbed WAB(DE) 2.7 and set it up with the widget from @kgonzago latest branch. I am seeing the problem with the color picker that I saw when I initially stared to test on WAB(DE)2.6. Clicking in the Line Color color patch pops up the color-picker, but then it behaves strangely, the color indicator marker zooms up and down the scale, the picked color does not get applied. image.png This may be unrelated to the version, but to some difference in @kgonzago repo and WAB?

@kgonzago - note that the color picker seems well-behaved in Firefox. That oddness happened in Chrome.

BobBooth commented 6 years ago

Testing on WAB(DE) 2.7 I am seeing the Origin and Cumulative construction methods work in the same way (but with the same problems for usability) that I saw on 2.8.

BobBooth commented 6 years ago

Kevin showed me that this was a problem with localization of labels making the app miss the tools. He is fixing.

kgonzago commented 6 years ago

Fixed in PR

kgonzago commented 6 years ago

FYI: ColorPicker issue is a known issue with Dojo 1.9 and not with WAB team.

BobBooth commented 6 years ago

I have verified that the missing UI elements are back.

lfunkhouser commented 6 years ago

@BobBooth Can you give more detail to the environment you are using to verified that the UI elements are back - on 2.8? 2.7? 2.6? Which browsers were tested?

BobBooth commented 6 years ago

@lfunkhouser - tested on 2.8 and 2.7 (most recently on 2.7). Tested on Chrome. @kgonzago talked me through editing a few strings in one of the js files when I initially found the issue, testing on 2.8. That fixed the problem. Kevin then made the fix in the repo and provided the code in his PR listed above. It now works.

BobBooth commented 6 years ago

Have now also tested on FireFox with WAB2.7. UI elements are back and working correctly on FF too.

BobBooth commented 6 years ago

image.png

BobBooth commented 6 years ago

PR submitted to update checklist test: https://github.com/ArcGIS/solutions-defense-test-catalog/pull/88

BobBooth commented 6 years ago

So, where this stands is, the code is now merged, test checklist updated. I found another issue that @adgiles and @kgonzago are looking at: https://app.zenhub.com/workspace/o/esri/distance-direction-addin-dotnet/issues/514 Will test any available fix Friday am, before I head out.

dfoll commented 6 years ago

I believe this can be closed. The documentation is in WAB and WAB DE here https://developersdev.arcgis.com/web-appbuilder/guide/widget-distance-and-direction.htm http://docdev.arcgis.com/en/web-appbuilder/create-apps/widget-distance-and-direction.htm

dfoll commented 6 years ago

moved back to sprint 4 because I realize I never updated the checklist test. Removed Doc-CMS because the workflow has been updated. Removing automated test labels because we don't have automated tests labels because we are not doing any automated testing with the widgets. Removed Defense label because it doesn't do anything for us in a repo that is only ours ... recommend removing label from repo

adgiles commented 6 years ago

@dfoll, @bobbooth has a PR above with the updated checklist test

dfoll commented 6 years ago

@BobBooth I've got your two pull requests for the widget in now.

dfoll commented 6 years ago

verified @BobBooth checklist test is good. I checked the Doc @topowright wrote for the widget... removing both those tags