#27016 closed task (blessed) (fixed)
Make placeholders/MCE views for Audio/Video shortcodes
Reported by: | wonderboymusic | Owned by: | |
---|---|---|---|
Milestone: | 3.9 | Priority: | normal |
Severity: | normal | Version: | 3.6 |
Component: | Media | Keywords: | has-patch |
Focuses: | javascript | Cc: |
Description
This is an offshoot of #26628, which was doing too much in the patch.
Attachments (8)
Change History (44)
This ticket was mentioned in IRC in #wordpress-dev by azaozz. View the logs.
11 years ago
#3
@
11 years ago
- Keywords has-patch added; needs-refresh removed
- Milestone changed from Awaiting Review to 3.9
#4
@
11 years ago
27016.2.diff restores this to its previously working state with some of the TinyMCE init
extras. I just realized you can't click to edit the shortcode. We should fix that as well. Perhaps in a later commit. I will leave this here until @azaozz or @gcorne weighs in.
#6
@
11 years ago
This is killer. Thank you. One caveat: the closing of the shortcode is optional. My test post that had the chromeless YouTube videos used shortcodes with no closer, so the placeholders didn't appear until I added closing tags.
#7
@
11 years ago
The idea is to eventually remove/refactor the 'wpgallery' TinyMCE plugin and (perhaps) make a 'wpmedia' plugin instead that will use wpViews.
We will also need another "single" mode in the media modal to handle editing of audio/video. Currently title, caption, description and embedding preference (link to or embed) can be set for media files that are in the library. Not sure what could be edited if the media file was added from external site.
One caveat: the closing of the shortcode is optional.
Do we want to support that or convert all shortcodes without a closing tag on editing? Looks like converting would be better.
#8
@
11 years ago
Good question - any shortcode inserted by editor would have closing, but when written by hand, I always exclude them.
#9
@
11 years ago
...when written by hand, I always exclude them.
Then the regex will be quite a bit more interesting. Will need a recursion or a loop to catch edge cases like [audio src="1.mp3"][audio src="2.mp3"][audio src="3.mp3"][/audio]
. Will fix it tomorrow.
#11
@
11 years ago
- Focuses javascript added
27016.5.diff gave me many headaches, but I think it kind of rocks now:
- Introduces MVC for audio and video shortodes/placeholders
- Placeholders for audio and video are now clickable and editable in the media modal
- The settings in the media modal map to new namespace defaults:
wp.media.audio.defaults
andwp.media.video.defaults
- Piggybacking on @gcornes amazing work - allows you to replace the audio or video in the media modal, it then updates your shortcode accordingly
- Adds the necessary back-and-forth in the TinyMCE
wpgallery
plugin, which is where all of our media parsing is living - Adds
media.model.PostVideo
andmedia.model.PostAudio
- Adds
media.controller.AudioDetails
andmedia.controller.VideoDetails
- Turns
media.controller.ReplaceImage
into a factory calledmedia.controller.ReplaceMedia
- Turns
media.view.MediaFrame.ImageDetails
into a factory calledmedia.view.MediaFrame.MediaDetails
- Turns
media.view.ImageDetails
into a factory calledmedia.view.MediaDetails
Without the factories, the amount of code duplication that would be necessary is breathtaking.
The image manipulation @gcorne is doing is about 3 million times more complicated than what I am doing - since it is altering existing HTML inline in TinyMCE. The code I am introducing is pretty good boilerplate for turning shortcodes into models into settings and back.
Will let this simmer for thoughts.
This ticket was mentioned in IRC in #wordpress-dev by wonderboymusic. View the logs.
11 years ago
#13
@
11 years ago
The functionality looks to be a good addition, although it would be nice if the modal also included a player above the various settings in the details view so that the user could preview the audio or video file.
In terms of the code, as I tried to illustrate in 26631.15.patch, I am not super fond of having a function that generates constructors. I think it is better for the particular instance to take options that change the behavior of the instance or to declare separate constructors and use either composition, mixins, or inheritance (ordered in terms of preference) to keep things DRY.
#14
@
11 years ago
I am going to work up a patch that has un-abstracted code in media-views.js
, then we can do an autopsy on it.
#16
@
11 years ago
There is a super-obnoxious bug, not immediately obvious - related to #26779:
- If you have a video shortcode, open the modal by clicking the placeholder, close the modal, open the modal, click play: the video is in a loading state forever
- If you have a video shortcode, open the modal by clicking the placeholder, play the video, close the modal, open the modal, click play: it works
I think there are 2 instances of an object pointing at the same local video, for some reason playing it does something special: looking into it.
#33
@
11 years ago
The "There are no associated subtitles" string from [27481] is not translatable. I've added a patch.
In 27097: