-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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#loadImage
method and Map#addImage
examples
#4478
Conversation
Map#loadImage
method and "Map#addImage" examplesMap#loadImage
method and Map#addImage
examples
docs/_posts/examples/.eslintrc
Outdated
@@ -5,7 +5,8 @@ | |||
"MapboxGeocoder": true, | |||
"MapboxDirections": true, | |||
"turf": true, | |||
"d3": true | |||
"d3": true, | |||
"Uint8Array": true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you remember why we have es6
env set to false
a few lines below? As far as I remember, turning this back to true
would allow not specifying common globals like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As best I remember we were just to avoid using es6 features with limited browser support. This is less of a concern now. I have enabled the "es6" flag and removed this global in the latest commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to continue to write examples without using ES6 features such as let
/const
or arrow functions (not supported by iOS 9) and for...of
(not supported by IE 11). I think it's useful to keep this configuration option to catch accidental uses of these types of features. I suggest we revert the latest commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right -- the examples don't get bubleify
ied. I'll revert the last commit and leave a comment about this in the eslint file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jfirebaugh as far as I remember, the env
switches are only for predefined globals like Uint8Array
— they're not used for catching keywords like let
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, you're right. And "parserOptions": { "ecmaVersion": 5 }
doesn't work either. 😞
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting these together @lucaswoj!
These look great, but I wonder if we could make our terminology more consistent/clear? Should the methods themselves be called "addSprite" (I realize it's a little late to be making this suggestion 😬)? and should these examples refer to sprites instead of icons
?
icon-
specifically refers to symbol layers in the style-spec, but these icons
can also be used by -pattern
layers as well, right?
@mollymerp There's a bunch of context on the naming decisions here: mapbox/mapbox-gl-style-spec#220 (comment) and #2059 (comment) There's a case to be made that these examples should be titled "Add an image to the map" but I think the current names will best direct those looking for this functionality. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐑
This PR:
ajax#getImage
asMap#loadImage
for use withMap#addImage
Map#addImage
with an external URLMap#addImage
with a procedurally generated imageLaunch Checklist