Skip to content

Implement toBlob function on ModelViewerElementBase#861

Merged
cdata merged 12 commits intogoogle:masterfrom
maaslalani:blobs
Nov 5, 2019
Merged

Implement toBlob function on ModelViewerElementBase#861
cdata merged 12 commits intogoogle:masterfrom
maaslalani:blobs

Conversation

@maaslalani
Copy link
Copy Markdown
Contributor

Reference Issue

Fixes #858

Adds toBlob to the ModelViewerElementBase.

https://developer.mozilla.org/en-US/docs/Web/API/HTMLCanvasElement/toBlob

General Approach

The general approach was to delegate the toBlob on the model-viewer to the canvas. Instead of using callbacks like HTMLCanvasElement does with toBlob, I used promises so that it is possible to use async/await syntax. We can change this back if needed.

Copy link
Copy Markdown
Contributor

@cdata cdata left a comment

Choose a reason for hiding this comment

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

Great work, thanks for the contribution @maaslalani !

Mostly minor feedback.

@cdata
Copy link
Copy Markdown
Contributor

cdata commented Nov 1, 2019

@maaslalani maaslalani requested a review from cdata November 4, 2019 22:34
@cdata
Copy link
Copy Markdown
Contributor

cdata commented Nov 5, 2019

Thanks for the great work on this @maaslalani LGTM 💯

@cdata cdata merged commit ba7fbe7 into google:master Nov 5, 2019
@cdata cdata added this to the v0.8.0 milestone Dec 19, 2019
elalish pushed a commit to elalish/model-viewer that referenced this pull request Feb 4, 2020
* Implement toBlob function on model-viewer-base

* Update documentation to specify qualityArgument is only available on Chrome Desktop and Firefox

* Prefer Url over URL casing

* Add License, Descriptive comment, and adjusted style guide to match other utility functions.

* Utility function dataUrlToBlob async

* Use spy helper to emulate unsupported browser and add test for equivalency between supported and unsupported browsers

* Add support for msToBlob for IE

* Remove .only from test suite

* Compare arrayBuffers

* Try and catch errors because spy toBlob will throw on edge

* Skip test on IE11 and add comment on using Response

* Remove https?: from objectUrlMatcher
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.

Improving the screenshot API using toBlob instead of toDataURL

3 participants