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

protocol proxy does not work with feature reader #250

Merged
merged 1 commit into from
Apr 1, 2014

Conversation

bartvde
Copy link
Member

@bartvde bartvde commented Mar 28, 2014

Opening up a ticket so I don't forget about this, will do PR later, but when using the protocol proxy with a feature reader things don't work. The trick in this case is that the response object has a features array and that needs to be passed in to the reader so e.g.:

var result = o.reader.read(response.features || response);

@bartvde
Copy link
Member Author

bartvde commented Mar 28, 2014

PR attached for this one. Tested in an application.

@bartvde
Copy link
Member Author

bartvde commented Mar 31, 2014

@marcjansen @chrismayer do you have time for a review on this 1 line patch?
In GeoExt 1 we had this handled at this level: https://github.com/geoext/geoext/blob/master/lib/GeoExt/data/FeatureReader.js#L70:L72

@marcjansen
Copy link
Member

I can review this, but I think it would be very important to have a test and or example showing the (then working) example. Do you agree @bartvde?

@marcjansen
Copy link
Member

It looks sane, don't get me wrong, but I want this to be a tested and known behaviour.

@bartvde
Copy link
Member Author

bartvde commented Mar 31, 2014

okay I'll see if I can add a testcase

@marcjansen
Copy link
Member

Thankee-sai.

@bartvde
Copy link
Member Author

bartvde commented Apr 1, 2014

@marcjansen test case added. TIA for any review.

@marcjansen
Copy link
Member

👍 Thanks Bart, please merge.

bartvde pushed a commit that referenced this pull request Apr 1, 2014
protocol proxy does not work with feature reader (r=@marcjansen)
@bartvde bartvde merged commit 9ba978e into geoext:master Apr 1, 2014
@bartvde bartvde deleted the protocolproxy branch April 1, 2014 10:13
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.

2 participants