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

GL JS docs copyedits #11175

Merged
merged 10 commits into from
Oct 27, 2021
Merged

GL JS docs copyedits #11175

merged 10 commits into from
Oct 27, 2021

Conversation

domlet
Copy link
Contributor

@domlet domlet commented Oct 25, 2021

Launch Checklist

  • briefly describe the changes in this PR
  • include before/after visuals or gifs if this PR includes visual changes
  • write tests for all new functionality
  • document any changes to public APIs
  • post benchmark scores
  • manually test the debug page
  • tagged @mapbox/map-design-team @mapbox/static-apis if this PR includes style spec API or visual changes
  • tagged @mapbox/gl-native if this PR includes shader changes or needs a native port
  • apply changelog label ('bug', 'feature', 'docs', etc) or use the label 'skip changelog'
  • add an entry inside this element for inclusion in the mapbox-gl-js changelog: <changelog></changelog>

@domlet domlet added docs 📜 skip changelog Used for PRs that do not need a changelog entry labels Oct 25, 2021
@domlet domlet changed the title Db itsthelittlethings GL JS docs copyedits Oct 25, 2021
Copy link
Contributor

@ryanhamley ryanhamley left a comment

Choose a reason for hiding this comment

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

What dependencies changed? I don't see changes to package.json anywhere, just the lock file.

src/ui/events.js Outdated Show resolved Hide resolved
src/ui/events.js Outdated Show resolved Hide resolved
src/ui/events.js Outdated Show resolved Hide resolved
src/ui/map.js Outdated Show resolved Hide resolved
src/ui/map.js Outdated
* @param {number} [options.zoom=0] The initial zoom level of the map. If `zoom` is not specified in the constructor options, Mapbox GL JS will look for it in the map's style object. If it is not specified in the style, either, it will default to `0`.
* @param {number} [options.bearing=0] The initial bearing (rotation) of the map, measured in degrees counter-clockwise from north. If `bearing` is not specified in the constructor options, Mapbox GL JS will look for it in the map's style object. If it is not specified in the style, either, it will default to `0`.
* @param {number} [options.pitch=0] The initial pitch (tilt) of the map, measured in degrees away from the plane of the screen (0-85). If `pitch` is not specified in the constructor options, Mapbox GL JS will look for it in the map's style object. If it is not specified in the style, either, it will default to `0`.
* @param {LngLatLike} [options.center=[0, 0]] The inital geographical [centerpoint](https://docs.mapbox.com/help/glossary/camera#center) of the map. If `center` is not specified in the constructor options, Mapbox GL JS will look for it in the map's style object. If it is not specified in the style, either, it will default to `[0, 0]` Note: Mapbox GL uses longitude, latitude coordinate order (as opposed to latitude, longitude) to match GeoJSON.
Copy link
Contributor Author

@domlet domlet Oct 26, 2021

Choose a reason for hiding this comment

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

Great catches @ryanhamley – correcting these issues now. Using #11072 as a reference for OGC:CRS84 and the GeoJSON standard for coordinate order.

Suggested change
* @param {LngLatLike} [options.center=[0, 0]] The inital geographical [centerpoint](https://docs.mapbox.com/help/glossary/camera#center) of the map. If `center` is not specified in the constructor options, Mapbox GL JS will look for it in the map's style object. If it is not specified in the style, either, it will default to `[0, 0]` Note: Mapbox GL uses longitude, latitude coordinate order (as opposed to latitude, longitude) to match GeoJSON.
* @param {LngLatLike} [options.center=[0, 0]] The initial geographical [center](https://docs.mapbox.com/help/glossary/camera#center) point of the map. If `center` is not specified in the constructor options, Mapbox GL JS will look for it in the map's style object. If it is not specified in the style, it will default to `[0, 0]` Note: Mapbox GL uses `longitude, latitude` coordinate order (as opposed to `latitude, longitude`) to match the [GeoJSON specification](https://datatracker.ietf.org/doc/html/rfc7946#section-4), which is equivalent to the OGC:CRS84 coordinate reference system.

src/ui/events.js Outdated Show resolved Hide resolved
src/ui/events.js Outdated Show resolved Hide resolved
@domlet
Copy link
Contributor Author

domlet commented Oct 27, 2021

Thanks @ryanhamley for your careful review! I've incorporated your feedback.

I don't see changes to package.json anywhere, just the lock file.

The lock file is no longer in the diff for this PR, I ran yarn install and tests now run (and pass) locally but not here. 😞

Any advice on resolving the 1 failing check?

@ryanhamley
Copy link
Contributor

Any advice on resolving the 1 failing check?

That's a known issue; the test is just flaky. I'll re-run those tests.

@ryanhamley ryanhamley merged commit 2f2e947 into main Oct 27, 2021
@ryanhamley ryanhamley deleted the db-itsthelittlethings branch October 27, 2021 21:36
ansis pushed a commit that referenced this pull request Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs 📜 skip changelog Used for PRs that do not need a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants