Video block: Gracefully auto-transform into Embed if embeddable resource provided #4990
Conversation
Fixes #4922. If an error is detected in a video block's video element, check whether the provided source is actually an embeddable resource. If so, convert the failed video block to an embed one.
mcsf
added
[Type] Question
| } ); | ||
|
|
||
| console.log( 'tryEmbed; querying' ); | ||
| window.fetch( apiURL, { credentials: 'include' } ).then( |
There was a problem hiding this comment.
Should we transform this to class component to avoid calling setState on unmounted components?
There was a problem hiding this comment.
Either that, or we source a bound fetch like:
|
Waiting for an UX/design review to do a more in-depth code review. |
|
This seems like an impressive enhancement, though it's a little difficult for me to understand exactly what goes on. Perhaps you can elaborate a bit? What seems to happen is that even if I try to embed an invalid URL, it creates an embed of sorts. It would seem more obvious to me if we showed an error here, perhaps. I noticed an issue, btw, not sure if this is both on master and in this branch:
Steps to reproduce:
The second time you insert a video block, it's born with the URL you pasted previously. |
|
Thanks for having a look at this, @jasmussen, despite my inability to get back to you. :) The parent issue has been closed, so I'm dropping this one. |
Partly fixes #4922. This is an experimental take on the parent issue which eschews the display of failure information. More than anything else, it needs feedback on whether it's a feature worth having, and—if so—it needs design feedback. Moreover, adding this automated correction behavior doesn't exclude displaying failure information if we can't handle the video automatically.
Description
When adding a new Video block, if the URL provided doesn't correspond to a usable video file (or if its loading otherwise fails) and the URL corresponds to an embeddable resource, then automatically transform the block into a proper Embed block.
How Has This Been Tested?
Try adding a Video block with the following kinds of values:
https://upload.wikimedia.org/wikipedia/commons/9/96/Curiosity%27s_Seven_Minutes_of_Terror.ogv)https://upload.wikimedia.org/wikipedia/commons/9/96/Curiosity%27s_Seven_Minutes_of_Terror.og)https://youtube.com/watch?v=lNHhmz-smlE)https://youtube.com/watch?v=lNHhmz-sml)Verify that the block is only converted to Embed when provided a valid embeddable URL.
Types of changes
Tentative UX enhancement
A technical aside
This pull request refactors the Video block's
editmethod into a functional component. For this, it extendedwithStatein order to accommodate its use case, allowing prop-dependent state initializations. If this is a desirable addition, I can spawn a new PR for it.Because of the refactor, the Commits view may provide diffs that are easier to review.
Checklist: