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

Improve documentation for ol.Feature #2378

Merged
merged 1 commit into from
Jul 16, 2014

Conversation

pagameba
Copy link
Member

This adds a bit more substance to the ol.Feature documentation.

* By default, the geometry property name is `geometry`, but this may be
* changed at any time using the `setGeometryName` method. When the geometry
* of a feature is requested, the current geometry property name is used so
* that rendered geometry associated with a feature may be changed on the fly.
Copy link
Member

Choose a reason for hiding this comment

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

This is a tough thing to explain. What do you think about something like this:

Typically, a feature has a single geometry property. You can set this with the setGeometry method and get it with getGeometry. It is possible to store more than one geometry on a feature. By default, the geometry used for rendering is set as a property named 'geometry'. If you want to use another geometry property for rendering, use the setGeometryName method.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is possible to store more than one geometry on a feature

what's the use case(s) for this? It's always easier to explain things with examples. This departs from the concept of features in formats like GeoJSON or KML

Copy link
Member

Choose a reason for hiding this comment

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

e.g. a polygon feature with its center point for labeling explicitly set in the feature as a second geometry

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, then I think it would be helpful for that (or other) examples to be given in the docs

Copy link
Member Author

Choose a reason for hiding this comment

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

I know this is the theory behind how this works, but is it actually possible with the current architecture to use more than one geometry simultaneously - to draw a polygon and label its center point (where the center point is provided by a second geometry)? I haven't seen anything that leads me to think this is possible right now. It might be nice if a layer or style could be configured with a geometry property name.

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure but GML supports it and our GML reader supports it as well

@pagameba pagameba added the doc label Jul 15, 2014
@pagameba pagameba self-assigned this Jul 15, 2014
@pagameba pagameba added this to the v3.0.0-gamma.3 milestone Jul 15, 2014
* for rendering, use the `setGeometryName` method to change the attribute
* property associated with the geometry for the feature. For example:
*
* var feature = new ol.Feature({
Copy link
Contributor

Choose a reason for hiding this comment

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

put this in a js backtick block

Copy link
Member Author

Choose a reason for hiding this comment

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

Indenting 4 spaces has the same effect I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

adding 'js' gives you highlighting - tho I agree that's not a big deal :-)

@pagameba
Copy link
Member Author

Thanks for the review @probins, all good catches!

@probins
Copy link
Contributor

probins commented Jul 15, 2014

otherwise, LGTM

@pagameba
Copy link
Member Author

I've changed the example to use backticks for js source highlighting as suggested by @probins, will merge when travis is happy.

pagameba added a commit that referenced this pull request Jul 16, 2014
Improve documentation for ol.Feature
@pagameba pagameba merged commit 38b36a0 into openlayers:master Jul 16, 2014
@pagameba pagameba deleted the doc-feature branch July 16, 2014 00:36
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.

None yet

4 participants