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

Fixed position of documentation.js '//addToMap' tag #857

Merged
merged 7 commits into from
Jul 24, 2017

Conversation

stebogit
Copy link
Collaborator

Ref. Turf-www #113

Updated @examples doc for updating the website

@stebogit stebogit requested a review from rowanwins July 22, 2017 09:58
Copy link
Member

@rowanwins rowanwins left a comment

Choose a reason for hiding this comment

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

If you can keep the properties below the //addToMap that would be my preference :)

*
* //addToMap
* curved.properties = { stroke: '#0f0' };
* var addToMap = [line, curved]
*/
Copy link
Member

Choose a reason for hiding this comment

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

so rather than moving the curved.properties above the //addToMap tag, I generally move it to below the var addToMap = [line, curved]. That benefit of that is that it isn't rendered as part of the code sample (which might potentially confuse people)

So I'd make it look like

  * var curved = turf.bezier(line);
   *
   * //addToMap
   * var addToMap = [line, curved];
   * curved.properties = { stroke: '#0f0' };
   */

Copy link
Member

Choose a reason for hiding this comment

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

👍 Agreed, so anything that's below//addToMap is not displayed in the website.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will do! 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@DenisCarriere @rowanwins ok the code after addToMap = ... is not rendered in the map on the website. So what's the purpose of adding properties in the example, which highlight the result on the map, if they are not actually used for the map?

Copy link
Member

Choose a reason for hiding this comment

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

There's two parts, the map which uses the addToMap as a featureCollection with all the properties. The second is the JSDocs example being rendered on the website which does not include the addToMap details.
Most of the extra properties added is to properly visualize the map example.

Hopefully that helps answer your question

@DenisCarriere
Copy link
Member

Just pushed the latest @example tests to this PR, we should be able to detect those JSDocs example errors a lot easier now.

@DenisCarriere
Copy link
Member

DenisCarriere commented Jul 22, 2017

Ok fixed most of the major @example errors.

I've excluded some of the new modules (6 skipped) from the tests since they haven't been added to @turf/turf yet.

$ tap test.example.js
test.example.js ................................... 100/106
  Skipped: 6
    clustersDbscan
    clustersKmeans
    getCluster
    clusterEach
    clusterReduce
    interpolate

total ............................................. 100/106

  100 passing (1s)
  6 pending

@DenisCarriere
Copy link
Member

@rowanwins 👍 Updated the addToMap JSDocs.

@DenisCarriere DenisCarriere merged commit 8b8d912 into master Jul 24, 2017
@DenisCarriere DenisCarriere deleted the doc-changes branch July 24, 2017 03:44
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.

3 participants