Closed anthonyburchell closed 6 years ago
What about other CSS & JS? Yesterday and the day before that I was working on modifying https://github.com/WordPress/gutenberg/pull/4710 There's a packaged plugin, a linked repo, several notes and screenshots
It's not a react component like what I believe this is suggesting, but it might be a faster route to market for now.
@Lewiscowles1986 I think that's what this will allow us. I spent the night getting the mediaelement
stuff separated from #805 If you look at the way the component renders, you'll see that we can expand on the layouts and how this displays.
https://github.com/anthonyburchell/gutenberg/blob/add/5240-mediaelement-component/components/media-element/index.js
I just got this repo in a state where the audio block is updated and utilizing the component. If you have a chance I could use the testing and input. I'm a bit tired and am learning js/react as I go so there are bound to be plenty of obvious mistakes ;) : https://github.com/anthonyburchell/gutenberg/tree/add/5240-mediaelement-component
My goal of this issue is for us to, at the very least, get the mediaelement
component exposed and have the audio (as it works today) utilizing it. From there we could expand to different templates/styles in the mediaelement
component. For instance, I am already working on the way that this component will handle playlist rendering. All of the object attributes for album art (images), meta, and other atts are now exposed to the player via the mediaItem
object.
would you by chance be using git pull
to integrate changes rather than git fetch
and git rebase -i {from}
?
It's quite hard to read through the changes without squashing commits at > 2-5 commits.
https://github.com/anthonyburchell/gutenberg/blob/add/5240-mediaelement-component/package-lock.json
for example has some artefacts from merging gone awry but most importantly it means lots of files have been touched that shouldn't be (I do this sometimes too)
Opening in Atom <ctrl>+<shift>+<f>
then two <<
as the find generally helps me to see what's changed so I can git patch without noise and then reapply from scratch.
@Lewiscowles1986 I totally did git pull and messed everything up. Probably going to start from scratch and re apply my changes later today. Thanks for catching that!
Sorry to hear that bud, happens to everyone. Let me know when it's done and I'll test, attempt to fix tests (I'm 0/1 on jest so far lol)
@Lewiscowles1986 I think this fixes it! New branch (ignore the number...I'm dyslexic and totally swapped them): https://github.com/anthonyburchell/gutenberg/tree/add/mediaelement-component-5420
Git is insanely aggravating. Let me know if this works :)
When I load an existing Gutenberg post
TypeError: Cannot read property 'url' of undefined
at save (http://localhost/wp-content/plugins/gutenberg-ab/blocks/build/index.js?ver=1519583973:36839:20)
at getSaveElement (http://localhost/wp-content/plugins/gutenberg-ab/blocks/build/index.js?ver=1519583973:19851:16)
at BlockListBlock.render (http://localhost/wp-content/plugins/gutenberg-ab/editor/build/index.js?ver=1519583973:24248:82)
at h (http://localhost/wp-content/plugins/gutenberg-ab/vendor/react-dom.min.3583f8be.js:130:364)
at beginWork (http://localhost/wp-content/plugins/gutenberg-ab/vendor/react-dom.min.3583f8be.js:134:70)
at d (http://localhost/wp-content/plugins/gutenberg-ab/vendor/react-dom.min.3583f8be.js:158:393)
at f (http://localhost/wp-content/plugins/gutenberg-ab/vendor/react-dom.min.3583f8be.js:159:214)
at g (http://localhost/wp-content/plugins/gutenberg-ab/vendor/react-dom.min.3583f8be.js:159:462)
at t (http://localhost/wp-content/plugins/gutenberg-ab/vendor/react-dom.min.3583f8be.js:167:3)
at r (http://localhost/wp-content/plugins/gutenberg-ab/vendor/react-dom.min.3583f8be.js:164:352)
<!-- wp:paragraph -->
<p>something something </p>
<!-- /wp:paragraph -->
<!-- wp:embed {"url":"https://www.youtube.com/watch?v=Pf9v9BUusHM","type":"video","providerNameSlug":"youtube"} -->
<figure class="wp-block-embed is-type-video is-provider-youtube">
https://www.youtube.com/watch?v=Pf9v9BUusHM
</figure>
<!-- /wp:embed -->
<!-- wp:shortcode -->
[video mp4="http://localhost/wp-content/uploads/2017/12/BigBuckBunny_320x180.mp4"] https://www.youtube.com/watch?v=Pf9v9BUusHM [audio mp3="http://localhost/wp-content/uploads/2017/12/bensound-buddy.mp3"]
<!-- /wp:shortcode -->
<!-- wp:video -->
<figure class="wp-block-video"><video controls="" src="http://localhost/wp-content/uploads/2017/12/BigBuckBunny_320x180.mp4"></video></figure>
<!-- /wp:video -->
<!-- wp:audio -->
<figure class="wp-block-audio"><audio controls="" src="http://localhost/wp-content/uploads/2017/12/bensound-buddy.mp3"></audio></figure>
<!-- /wp:audio -->
<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->
It's also not rendering on a blank page or post.
Thanks for the details! I'm looking into it. I think it's a matter of the url attribute being saved as src or vice versa. Also might be something I did when manually bringing over my changes when I was wrangling git prior. Will report back soon!
My mediaelement-component repo is up to date again @Lewiscowles1986 and the edit/save issues should be solved. I'm hitting one last snag now that I'm very much stuck on. For whatever reason, in the editor, the player renders as expected. But on the front end, it seems that the mediaelement scripts are not getting enqueued. I added mediaelement
to the wp-components
array in client-assets.php
but I have a suspicion this is not being respected on the front end. Any thoughts anyone has would be most helpful. :)
Hey buddy, it's triggering some ci errors. One from a test, and a heap of code-style.
blocks/test/full-content.js
full post content fixture › core__audio
[1]
[1] File 'core__audio.json' does not match expected value:
[1]
[1] expect(received).toEqual(expected)
[1]
[1] Expected value to equal:
[1] [{"attributes": {"align": "right", "caption": [], "src": "https://media.simplecast.com/episodes/audio/80564/draft-podcast-51-livePublish2.mp3"}, "innerBlocks": [], "isValid": true, "name": "core/audio", "originalContent": "<figure class=\"wp-block-audio alignright\">
[1] <audio controls=\"\" src=\"https://media.simplecast.com/episodes/audio/80564/draft-podcast-51-livePublish2.mp3\"></audio>
[1] </figure>", "uid": "_uid_0"}]
[1] Received:
[1] [{"attributes": {"align": "right", "caption": [], "src": "https://media.simplecast.com/episodes/audio/80564/draft-podcast-51-livePublish2.mp3"}, "innerBlocks": [], "isValid": false, "name": "core/audio", "originalContent": "<figure class=\"wp-block-audio alignright\">
[1] <audio controls=\"\" src=\"https://media.simplecast.com/episodes/audio/80564/draft-podcast-51-livePublish2.mp3\"></audio>
[1] </figure>", "uid": "_uid_0"}]
blocks/library/audio/index.js
[0] 18:8 error 'RichText' is defined but never used no-unused-vars
[0] 37:20 error Missing trailing comma comma-dangle
[0] 42:26 error Missing trailing comma comma-dangle
[0] 83:12 error 'align' is assigned a value but never used no-unused-vars
[0] 83:19 error 'caption' is assigned a value but never used no-unused-vars
[0] 83:32 error 'album' is assigned a value but never used no-unused-vars
[0] 83:39 error 'artist' is assigned a value but never used no-unused-vars
[0] 83:47 error 'image' is assigned a value but never used no-unused-vars
[0] 83:54 error 'title' is assigned a value but never used no-unused-vars
[0] 99:2 error Mixed spaces and tabs no-mixed-spaces-and-tabs
[0] 101:25 error Missing trailing comma comma-dangle
[0] 104:6 error Unexpected console statement no-console
[0] 183:14 error Missing trailing comma comma-dangle
[0] 190:11 error 'align' is assigned a value but never used no-unused-vars
[0] 190:23 error 'album' is assigned a value but never used no-unused-vars
[0] 190:30 error 'artist' is assigned a value but never used no-unused-vars
[0] 190:38 error 'id' is assigned a value but never used no-unused-vars
[0] 190:42 error 'image' is assigned a value but never used no-unused-vars
[0] 190:49 error 'title' is assigned a value but never used no-unused-vars
[0] 190:56 error 'caption' is assigned a value but never used no-unused-vars
components/media-element/index.js
[0] 6:53 error Block must not be padded by blank lines padded-blocks
[0] 13:9 error There must be a space inside this paren space-in-parens
[0] 13:23 error 'instance' is defined but never used no-unused-vars
[0] 13:31 error There must be a space inside this paren space-in-parens
[0] 17:7 error There must be a space inside this paren space-in-parens
[0] 17:8 error 'media' is defined but never used no-unused-vars
[0] 17:13 error There must be a space inside this paren space-in-parens
[0] 21:11 error Block must not be padded by blank lines padded-blocks
[0] 25:4 error 'sources' is assigned a value but never used no-unused-vars
[0] 25:24 error There must be a space inside this paren space-in-parens
[0] 25:38 error There must be a space inside this paren space-in-parens
[0] 26:4 error 'tracks' is assigned a value but never used no-unused-vars
[0] 26:23 error There must be a space inside this paren space-in-parens
[0] 26:36 error There must be a space inside this paren space-in-parens
[0] 32:17 error Expected space(s) after '${' template-curly-spacing
[0] 32:34 error There must be a space inside this paren space-in-parens
[0] 32:35 error Strings must use singlequote quotes
[0] 32:39 error There must be a space inside this paren space-in-parens
[0] 32:40 error Expected space(s) before '}' template-curly-spacing
[0] 33:5 error Expected space(s) after '${' template-curly-spacing
[0] 33:22 error There must be a space inside this paren space-in-parens
[0] 33:23 error Strings must use singlequote quotes
[0] 33:27 error There must be a space inside this paren space-in-parens
[0] 33:28 error Expected space(s) before '}' template-curly-spacing
[0] 35:17 error Expected space(s) after '${' template-curly-spacing
[0] 35:27 error Expected space(s) before '}' template-curly-spacing
[0] 35:37 error Expected space(s) after '${' template-curly-spacing
[0] 35:50 error Expected space(s) before '}' template-curly-spacing
[0] 35:61 error Expected space(s) after '${' template-curly-spacing
[0] 35:75 error Expected space(s) before '}' template-curly-spacing
[0] 35:77 error Expected space(s) after '${' template-curly-spacing
[0] 35:79 error There must be a space inside this paren space-in-parens
[0] 35:104 error Expected space(s) after '${' template-curly-spacing
[0] 35:118 error Expected space(s) before '}' template-curly-spacing
[0] 35:125 error There must be a space inside this paren space-in-parens
[0] 35:126 error Expected space(s) before '}' template-curly-spacing
[0] 36:6 error Expected space(s) after '${' template-curly-spacing
[0] 36:8 error There must be a space inside this paren space-in-parens
[0] 36:42 error There must be a space inside this paren space-in-parens
[0] 36:43 error Expected space(s) before '}' template-curly-spacing
[0] 36:44 error Expected space(s) after '${' template-curly-spacing
[0] 36:46 error There must be a space inside this paren space-in-parens
[0] 36:74 error Expected space(s) after '${' template-curly-spacing
[0] 36:89 error Expected space(s) before '}' template-curly-spacing
[0] 36:97 error There must be a space inside this paren space-in-parens
[0] 36:98 error Expected space(s) before '}' template-curly-spacing
[0] 37:6 error Expected space(s) after '${' template-curly-spacing
[0] 37:17 error Expected space(s) before '}' template-curly-spacing
[0] 39:17 error Expected space(s) after '${' template-curly-spacing
[0] 39:27 error Expected space(s) before '}' template-curly-spacing
[0] 39:37 error Expected space(s) after '${' template-curly-spacing
[0] 39:50 error Expected space(s) before '}' template-curly-spacing
[0] 39:58 error Expected space(s) after '${' template-curly-spacing
[0] 39:69 error Expected space(s) before '}' template-curly-spacing
[0] 40:6 error Expected space(s) after '${' template-curly-spacing
[0] 40:17 error Expected space(s) before '}' template-curly-spacing
[0] 44:10 error There must be a space inside this paren space-in-parens
[0] 44:40 error A space is required after '{' react/jsx-curly-spacing
[0] 44:41 error A space is required after '{' object-curly-spacing
[0] 44:59 error A space is required before '}' object-curly-spacing
[0] 44:60 error A space is required before '}' react/jsx-curly-spacing
[0] 44:68 error There must be a space inside this paren space-in-parens
[0] 46:2 error Block must not be padded by blank lines padded-blocks
[0] 48:22 error Block must not be padded by blank lines padded-blocks
[0] 50:9 error A space is required after '{' object-curly-spacing
[0] 50:28 error A space is required before '}' object-curly-spacing
[0] 52:6 error There must be a space inside this paren space-in-parens
[0] 52:7 error Unary operator '!' must be followed by whitespace space-unary-ops
[0] 52:26 error There must be a space inside this paren space-in-parens
[0] 56:6 error There must be a space inside this paren space-in-parens
[0] 56:48 error There must be a space inside this paren space-in-parens
[0] 57:33 error There must be a space inside this paren space-in-parens
[0] 57:48 error There must be a space inside this paren space-in-parens
[0] 57:67 error There must be a space inside this paren space-in-parens
[0] 59:14 error There must be a space inside this paren space-in-parens
[0] 59:36 error There must be a space inside this paren space-in-parens
[0] 59:53 error There must be a space inside this paren space-in-parens
[0] 59:75 error There must be a space inside this paren space-in-parens
[0] 60:12 error There must be a space inside this paren space-in-parens
[0] 60:24 error There must be a space inside this paren space-in-parens
[0] 60:39 error There must be a space inside this paren space-in-parens
[0] 60:51 error There must be a space inside this paren space-in-parens
[0] 60:52 error Missing trailing comma comma-dangle
[0] 61:5 error There must be a space inside this paren space-in-parens
[0] 62:17 error There must be a space inside this paren space-in-parens
[0] 62:18 error A space is required after '{' object-curly-spacing
[0] 62:75 error A space is required before '}' object-curly-spacing
[0] 62:76 error There must be a space inside this paren space-in-parens
[0] 65:16 error There must be a space inside this paren space-in-parens
[0] 65:17 error A space is required after '{' object-curly-spacing
[0] 65:85 error A space is required before '}' object-curly-spacing
[0] 65:86 error There must be a space inside this paren space-in-parens
[0] 69:6 error There must be a space inside this paren space-in-parens
[0] 69:24 error There must be a space inside this paren space-in-parens
[0] 71:17 error There must be a space inside this paren space-in-parens
[0] 71:18 error A space is required after '{' object-curly-spacing
[0] 71:31 error A space is required before '}' object-curly-spacing
[0] 71:32 error There must be a space inside this paren space-in-parens
git clone https://github.com/anthonyburchell/gutenberg gutenberg-ab
cd gutenberg-ab
git checkout add/mediaelement-component-5420
composer update
nvm use 8
npm update
npm run ci
composer run lint
git checkout composer.lock package-lock.json
npm run package-plugin
I'm running in-browser tests atm
Thanks for that! I need to learn how to run these tests. Oh there it is in "Process" 😅 This is great info! Working on fixing these now.
It's running without any frontend console or debugging errors, which is aces. There was a problem loading one gutenberg post (which may be a false positive from testing features & block-types).
When adding to another post, video block comes without mediaelement-js
, but also without errors. Audio block comes with mediaelement-js
, but without effects from a plugin I've authored, that adds mediaelement-js
controls for playback speed.
There is a tiny CSS bug in chrome that results in the bottom of the duration displays being "cut-off"
Awesome news! So yes, I expected video to not be correct for the time being. The video block will probably need to be updated separately. The goal of this issue is to get mediaelement component exposed and have the audio block use it as an example so we can iterate later to style things with features like you have in your plugin. There is conditional logic in the component to accept a video mediaType
however the block you have currently is just rendering down to html5 instead of the mediaelement
format. Which would look something like this:
<MediaElement
id="player1"
mediaType="video"
preload="auto"
controls
width="640"
height="360"
poster=""
sources={JSON.stringify(mediaItems)}
options={JSON.stringify(options)}
tracks={JSON.stringify(mediaItems)}
/>
false positive post was failing to load audio block using the following code
<!-- wp:audio /-->
I'm unsure how that got there, but I've checked and Gutenberg does support the audioblock without error
@anthonyburchell – I spent some time looking into the front end issues here and I think some context about how WordPress works currently is going to help you get this sorted.
First of all, WordPress enqueues it's own custom wrapper + styles for MEJS only when necessary. Generally, this happens whenever a shortcode is being rendered that needs MEJS support. See this example from the audio shortcode.
The wp-mediaelement
script manages the initialization of MEJS on specific elements—specifically, those that have wp-audio-shortcode
or wp-video-shortcode
classes, which you can see in this section of the code.
I'm not sure how the audio and video blocks are being registered at the moment, but if we can make use of the render
method to enqueue wp-mediaelement
scripts and styles, and we add the correct classes in Core, then we should be good to go here.
@anthonyburchell @joemcgill as much as it may be a large hammer to crack a walnut, I still think #4710 has the simplest way to implement this via the sandbox component. As for front-end rendering, surely it'd be easier to add URL support to the audio and video shortcodes if they don't exist already and work with what is?
@Lewiscowles1986 If I'm understanding correctly, this would have us keeping audio/video embeds stored as shortcodes in Gutenberg, rather than creating first-class Gutenberg blocks. I think that might be an ok short term solution, but seems to me it would be better to implement these as blocks without the need for shortcodes.
I hear ya @joemcgill. My concern is that this will be yet another divergence that will confuse existing WP users and leave more options, more engineering effort. The only reason I pull most things into shortcodes is that it means
I do take your point though.
Current shortcodes should continue to work regardless, IMO. But any new audio/video embeds being created from the Gutenberg UI should have a first-class experience.
Perhaps there's a middle ground here where we could make use of the Gutenberg's PHP rendering callback to reuse some of the existing shortcode rendering logic for these blocks so we're not totally forking these elements?
Perhaps there's a middle ground here where we could make use of the Gutenberg's PHP rendering callback to reuse some of the existing shortcode rendering logic for these blocks so we're not totally forking these elements?
Yes, by registering a callback the original source effectively becomes a shortcode (just more functional out of the box). We just need to see which media attributes should be saved as attributes for better server-side handling (like ID).
@mtias I'm working on this today and have started down the route of using the callback for the playlist block. I think this is the best way to provide a certain level of backwards compatibility. I've gone two routes in my initial attempts to build the playlist block ( #805 ) The first attempt was using pure js but that breaks anything relying on mediaelement, which leads to this issue here being created. I don't think we actually need a mediaelement component for playlists at all, at least for the initial merge of Gutenberg. I think we should use it though for audio and video elements.
Going to close this as a mediaelement component is not needed anymore if we have php callbacks available.
Going to close this as a mediaelement component is not needed anymore if we have php callbacks available.
How to use these php callbacks, what they are? For now I am using this:
wp_enqueue_style( 'wp-mediaelement' );
wp_enqueue_script( 'wp-mediaelement' );
But I was interested why wasn't it working out-of-the-box for the audio block, that is how I got here. Is there any plans to make it work? And currently enqueuing this also enqueues some legacy files for support for IE 9, how can I opt out of this and only enqueue the newest file?
Honestly you don't need it or PHP callbacks in 2021. Try to re-imagine what you are doing.
For my own uses https://github.com/CODESIGN2/media-playback-speed now works without mediaelement-js or jQuery, as well as with for legacy systems (you need to provide your own controls to work with HTML5, but there is a declarative convention. It's in the plugin repo & WordPress plugin support channels).
An alternative if you must work with shortcodes within Gutenberg and have them visually render the frontend could be https://github.com/CODESIGN2/gutenberg-shortcode-preview-block . It renders any shortcode in all versions of WordPress I've tested it on, and uses some community code as well as fixups since 2017 to keep it working. Unfortunately I just don't have the time to keep this one alive and on plugin directory, potentially supporting a wide use-case. Numbers show it was never very popular, which also points to that being a niche need, but any shortcode based code that still exists could be used last I tested. It also has Storybook JS integration (degraded in last version bump) and tests.
One other reason I stopped maintaining it was that I now feel, perhaps 4 years on keeping shortcodes alive isn't a good or healthy thing. WordPress core team have enough without me sticking my oar into the mud trying to steer them towards things that honestly... Most customers don't need and shouldn't be a core concern.
It would be lovely to have the mediaelement JS pulled into a plugin if at-all. I would hope if anyone can do that it would allow Gutenberg blocks to be decorated with it... Playlist, video, audio blocks which are perhaps higher-order or just straight forks, which use ME.js. By working that way, it keeps CORE more thin and lean, but if the right ecosystem cropped-up around it, would enable our plugins to collaborate to deliver interesting experiences.
It would be lovely to have the mediaelement JS pulled into a plugin if at-all. I would hope if anyone can do that it would allow Gutenberg blocks to be decorated with it.
That's a good idea, I actually don't care about shortcodes I am only trying to style the audio block. After looking at audio element API I have decided to code my own player. I am willing to help coding a plugin for that, but I am a relatively new programmer and I don't think I could make it a versatile plugin myself, until now I was only coding plugins for single-site use. Btw. your little plugin works great for changing speed of playback 👍
Thanks @Micu22 When you say you want to code your own player, perhaps open a new repo and tag me in an issue, so we don't fill up mentions of people on this closed issue? it's @Lewiscowles1986
and I'll get notified of the tag.
Issue Overview
The current implementations of the video and audio blocks simply render out html. We need a
mediaelement
component to possibly allow backwards compatibility with the Coremediaelement
library and also allow us templates to expand on the functionality of these players. This issue was discovered while attempting to build out the Playlist block ( issue #805 ). In order to render the playlist in the same way core does we need to expose themediaelement
library through thewp
global. My current repo for issue #805 contains a somewhat working version of themediaelement
component and global. I am in the process of pulling that out and editing the video/audio blocks to utilize the component. I'll make a PR for the component here and we can start work on bettering the video and audio blocks later as mentioned in #4961Expected Behavior
Usage of the component is as follows
We can expand on this later to get more stylized and functional
Possible Solution
Working on getting the repo cleaned up and ready for PR
Screenshots / Video
Related Issues and/or PRs
805 #4961
My progress on this issue can be tracked at this repo: https://github.com/anthonyburchell/gutenberg/tree/add/mediaelement-component-5420
Todos