Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add queryFeatures() to send query requests to feature services #126

Merged
merged 7 commits into from
Feb 28, 2018

Conversation

tomwayson
Copy link
Member

@tomwayson tomwayson commented Feb 24, 2018

resolves #116

@coveralls
Copy link

coveralls commented Feb 24, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling ad61b0a on feat/query-features into 78512a3 on master.

@tomwayson
Copy link
Member Author

Demo:

arcgis-rest-feature-service-demo

@apfister
Copy link
Contributor

this looks awesome. how is paging normally handled for developers using services like this? is it worth having a convenience method like getAllFeatures() that will do the multiple requests through pages for you and just return all the data?

@tomwayson
Copy link
Member Author

@apfister you should open an issue for that. @jgravois also suggested adding it and pointed me to https://github.com/koopjs/featureservice/. We'd have to figure out the relationship between that lib and arcgis-rest-feature-service. One idea would be to make a PR over there to use arcgis-rest-feature-service, which should solve the issue about it not working in a browser.

Copy link
Contributor

@noahmulfinger noahmulfinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I saw a few minor typos. Also, it might be good to have some example tree types and conditions in the demo, either in the placeholder or somewhere else. Just to make it a little more clear

<div class="col-md-12">
<div class="jumbotron" >
<h1>search features!</h1>
<form id="searchFrom" class="form-inline">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be searchForm?

<script src="node_modules/@esri/arcgis-rest-feature-service/dist/umd/arcgis-rest-feature-service.umd.js"></script>

<script>
// respond when s user fills out he search text and hits enter
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typos here

@tomwayson
Copy link
Member Author

Thanks @noahmulfinger, I'll address your comments when I update this for #129.

@jgravois
Copy link
Contributor

@apfister see: Esri/geoservices-js#35

@apfister
Copy link
Contributor

@jgravois the noodling began back in 2013! let's put those noodles in the pot! ok, i'll see myself out now..

want to make sure query features demo works when other packages are
configured as externals and excluded from the UMD bundle
now that it is treated as external and not included in the bundle
@tomwayson
Copy link
Member Author

@apfister QueryToCsv for life yo!

@tomwayson tomwayson merged commit 2a30d22 into master Feb 28, 2018
@tomwayson tomwayson deleted the feat/query-features branch February 28, 2018 06:05
@tomwayson
Copy link
Member Author

@jgravois FYI - I have this working via npm link in cedar:

image

Let me know what I can do to help cut a release so I can show that live on Tues.

@jgravois
Copy link
Contributor

jgravois commented Mar 3, 2018

Let me know what I can do to help cut a release so I can show that live on Tues.

can you run the same exact test with the un-minified UMD build i just pushed up in #141? if all is well i'll merge that PR (and #140 and #143) and tag v1.1.0.

@tomwayson
Copy link
Member Author

Will do later this evening

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add demo for @esri/arcgis-rest-feature-service
5 participants