search

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

Inital Commit for the new Visibility widget #270

Closed kgonzago closed 6 years ago

kgonzago commented 6 years ago

Issue by lyle6003 Tuesday Feb 06, 2018 at 16:11 GMT Originally opened as https://devtopia.esri.com/WebGIS/arcgis-webappbuilder/pull/11759

@Juns6831 this is the initial commit for the Visibility widget created by Defense Solutions. The work for this widget can be tracked in the following issue:
https://devtopia.esri.com/WebGIS/arcgis-webappbuilder/issues/11701

lyle6003 included the following code: https://devtopia.esri.com/WebGIS/arcgis-webappbuilder/pull/11759/commits

kgonzago commented 6 years ago

Comment by Juns6831 Wednesday Feb 07, 2018 at 01:17 GMT

This widget includes some python and gdb files, can you remove them from the widget if they are not required when run the widget? The widget in WAB is supposed to include JS/CSS/HTML/MD files and some unit tests files.

kgonzago commented 6 years ago

Comment by lyle6003 Wednesday Feb 14, 2018 at 10:40 GMT

The un necessary files for this workflow have been removed.

kgonzago commented 6 years ago

Comment by lyle6003 Friday Feb 16, 2018 at 17:00 GMT

@Juns6831 We have submitted all code changes and things asked per the review meeting. Please let us know what the next step we need to take.

kgonzago commented 6 years ago

Comment by Juns6831 Thursday Feb 22, 2018 at 01:32 GMT

You'd better not use ${nls.abc} in data-dojo-props in HTML template because this may break in some languages.

kgonzago commented 6 years ago

Comment by Juns6831 Thursday Feb 22, 2018 at 01:34 GMT

Found you use jquery in your widget, and there is a jquery in stemapp/libs, can you check whether you can use it? If you use the lib in stemapp, your widget will be more lightweight.

kgonzago commented 6 years ago

@adgiles beijing dev team provided comments on the visibility widget commit. Correct me if I'm wrong, but didn't you address the jquery issue?

adgiles commented 6 years ago

yes I think so, jquery.knob.min.js is now:

define(['jimu/loaderplugins/jquery-loader! ./libs/jquery/jquery-1-11-1.js'],

Looking at the code on this repo the original jquery.min.js file is still here not sure what devtopia is like

kgonzago commented 6 years ago

I confirmed @adgiles did address these issues in this repo but changes have not been moved to Devtopia.

@dfoll @topowright @ACueva Can you push the latest changes for the Visibility widget to this existing PR?

topowright-zz commented 6 years ago

I will take a look at this today. Talking with @adgiles this morning I think I know what needs to be done. I might need help from from either @kgonzago or @adgiles this morning just to make sure nothing is missed.

topowright-zz commented 6 years ago

The files related to the gdb and the python script have been removed.

topowright-zz commented 6 years ago

@kgonzago I have opened a new issue that addresses the statements from the Dev team for the items that I think have not been addressed. https://github.com/Esri/visibility-addin-dotnet/issues/272

If this is already done then apologies for the mixup.

topowright-zz commented 6 years ago

Trying to close this issue and issue #272. If I am missing something please let me know.

topowright-zz commented 6 years ago

This issue has been addressed with the most recent updates to the PR on Devtopia.

topowright-zz commented 6 years ago

This work has been finsihed and pushed into WAB DE repo.

adgiles commented 6 years ago

@topowright The Visibility widget is now showing in the daily build