-
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 snapshot example #6846
add snapshot example #6846
Conversation
override default #map width simplified map center cosmetic button padding and box-shadow h1 y-align calc() includes offset for button padding copy to canvas previously copied to a div using HTML, now using a canvas. let to var the map variable on 36 was giving me a lint error (Parsing error: Unexpected token map) will try converting to var to see if that fixes it (?) let to var (complete) looks like that worked.. apparatenly the linter doesn't except let rm string literals ( ` not allowed by linter) but pass all other tests indents and semicolons more linting I'm glad that you rebase merge as opposed to basic merge preserveDrawingBuffer comment always show textbox attribution OSM (typo) semicolons title updated in accordance with: https://github.com/mapbox/mapbox-gl-js/tree/master/docs#writing-examples css snapshot canvas size using CSS to avoid a visual resizing of the canvas element (i.e. need to wait for window to load to get innerWidth and innerHeight). snapshot canvas resize on window load read mapbox logo from DOM logo (move up a few pixels) semicolon linting make it look presentable re-ordered #snapshot css rm comment about not working in FireFox because it now seems to work just fine update comments on OSM and Mapbox attribution use a left and a right div add clickMessage style in both CSS and JS (!) url encoded logo bugfix (firefox not drawing SVGs to canvas) some linting some more linting yet more linting there is an easier way to do this isn't there?
…simplify CSS and canvas text
44f23c4
to
5c543df
Compare
…simplify CSS and canvas text urg
5732949
to
27e0247
Compare
Sorry to do a little rewrite on the CSS. I think it looks good. Super cool demo. 👍 |
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.
shouldn't the new line end with 'right'.. that, is
..generate a static image on the right.
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 is a great example! Thanks for contributing it @samsammurphy.
The technique to get the logo SVG relies more heavily on implementation details than I'm comfortable endorsing in an official example -- it assumes that the logo control uses a background image with a url
value with specific formatting. A way to make this more robust would be for the logo control to expose a public getImage
method that returns an Image
object suitable for ctx.drawImage
.
@jfirebaugh I agree this is a bit too brittle to be comfortable with, but the |
While I agree relying on internal implementation details isn't a good idea, I think it's better to include this example as is and leave the door open to change/refine it later on. That aside, any kind of implementation of the Logo and Attribution, even a no so ideal one makes complying with the TOS much easier.
Can we not just add the |
I'm going to close this PR because we're uncomfortable with some of the implementation details here. @ChrisLoer and I are hoping to work on a video export feature soon and that could probably expose a snapshot method as well then we can revisit adding examples for these features. |
This has been handy in a few cases to demonstrate how to take a static snapshot of a map, hope it or a reincarnation of it does eventually make it into the examples at some point. |
Even if it gets changed later from a future video export feature or refactor, my 2c are it would be still useful for others to have this published in the meantime. |
this replaces #6518
@samsammurphy thanks for your contribution! I had to open a new PR because #6513 caused some of your work to break, and I didn't want to bother you further with more back and forth (I can't push to your branch). I also changed the styling around a bit and simplified the code a bit because we try to keep our examples as minimal as possible.
we'll have to be careful with this example because it will break if we change our CSS for the logo control at all 😕
Launch Checklist