-
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
Warn user if required sprite property is not present or non-existent image is requested from sprite #6831
Conversation
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.
nice! the style-spec validation tests are a little different from our other unit tests, and use fixtures.
here are the relevant fixtures for the missing glyphs validation:
https://github.com/mapbox/mapbox-gl-js/blob/master/test/unit/style-spec/fixture/missing-glyphs.input.json
https://github.com/mapbox/mapbox-gl-js/blob/master/test/unit/style-spec/fixture/missing-glyphs.output.json
we also want to be careful and make sure this error doesn't fully break existing styles that fail this validation check. from what I can tell right now it does block rendering if the validation fails on the initial style load, but it doesn't break if the validation fails with addLayer
or set*Property
8fc2c97
to
b5acbb5
Compare
b261bee
to
1a45039
Compare
…image is requested from sprite
1a45039
to
0498166
Compare
Closing in favor of #7987 |
@ryanhamley I wasn't aware of this pr, sorry. Do you think it makes sense to still add the warning for missing icons to the other pr? (in the case the user doesn't add the missing icon in the event's callback) |
No problem. It's something I quickly took a stab at awhile ago and then never went back to. The goal was always a more fleshed out option like this. A warning might be useful because right now you wouldn't necessarily know that anything was wrong if you weren't listening for the event. I can imagine spending a few hours tearing my hair out trying to figure out why my image wasn't showing up only to realize there's a typo or something. |
Launch Checklist
Closes #6823