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 map.getMaxBounds method #4890

Merged
merged 2 commits into from
Jun 27, 2017
Merged

add map.getMaxBounds method #4890

merged 2 commits into from
Jun 27, 2017

Conversation

andrewharvey
Copy link
Collaborator

@andrewharvey andrewharvey commented Jun 26, 2017

Launch Checklist

part of #4029

Thanks to @lamuertepeluda for the inspiration in #4029 (comment)

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • document any changes to public APIs
  • post benchmark scores
  • manually test the debug page

Credit to @lamuertepeluda which provided some of the basis to this
commit at #4029 (comment)
Copy link
Contributor

@mollymerp mollymerp left a comment

Choose a reason for hiding this comment

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

🙏 thanks @andrewharvey! just a small change but otherwise this looks good to me!

src/ui/map.js Outdated
* @returns {LngLatBounds | null} The maximum bounds the map is constrained to, or `null` if none set.
*/
getMaxBounds () {
if (!(!Array.isArray(this.transform.latRange) || this.transform.latRange.length === 0) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

I find this a little hard to read 😕 can we invert the check ( if (this.transform.latRange && this.transform.latRange.length == 2) && ... else { return null; } ) for clarity?

@@ -434,6 +450,7 @@ class Map extends Camera {
return this;

}

Copy link
Contributor

Choose a reason for hiding this comment

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

accidental line? 🙃

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

everywhere else had a whitespace between methods, so I added one here to be consistent.

@andrewharvey
Copy link
Collaborator Author

should be good now

Copy link
Contributor

@mollymerp mollymerp left a comment

Choose a reason for hiding this comment

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

👍

@andrewharvey andrewharvey merged commit 3499f75 into master Jun 27, 2017
@andrewharvey andrewharvey deleted the getMaxBounds branch June 27, 2017 00:46
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