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 Block: Playlist #805

Open
jasmussen opened this Issue May 16, 2017 · 32 comments

Comments

Projects
@jasmussen
Copy link
Contributor

jasmussen commented May 16, 2017

Splitting this out from #283. See also #804.

Attributes

  • None, Left, Right, Center, Full Width.
  • Caption

States

Placeholder:

playlist placeholder

Neutral:

playlist neutral

Selected:

playlist selected

@karmatosed

This comment has been minimized.

Copy link
Member

karmatosed commented Aug 2, 2017

Is the idea this is both video and audio? To make sure we have the same as existing editor we would need to have both video and audio playlists.

@StaggerLeee

This comment has been minimized.

Copy link

StaggerLeee commented Aug 3, 2017

I never liked how it force itself to use 100% article content width. And it is most used case an User will put it in the Post.

Playlist is perfect for sidebars, widgets. And maybe some list of podcasts in Posts. Not even then it looks nice enlarged to 100% width.

Playlist could use a bit of new philosophy and rewrite of code. I mean not necessary prevent 100% width, nothing wrong with it. But rearange things, elements, to make it more modern and fitting.

I dont know, bigger thumbnail at the left of song, not at top.
Some new elements, etc... Not necessary any element anyone so desperately needs. But just to make it appear better when say it is 700px width in the Post. (Or worst, today modern websites without sidebars, and imagine Playlist 1000px - 1200px wide)

@StaggerLeee

This comment has been minimized.

Copy link

StaggerLeee commented Aug 3, 2017

Here is how it looks compared to SoundCloud widgets. They use nothing extraordinary and revolutionary. Just different appearance and clever arrangements.

@karmatosed karmatosed added this to the Beta 0.8.0 milestone Aug 3, 2017

@mtias mtias modified the milestones: Beta 0.9.0, Beta 0.8.0 Aug 10, 2017

@mtias mtias modified the milestones: 0.10.0, Beta 0.9.0 Aug 18, 2017

@karmatosed karmatosed modified the milestones: 0.11.0, 0.10.0, Beta 1.2, Beta 1.1 Aug 24, 2017

@mtias mtias modified the milestones: Beta 1.2, Beta 1.1 Sep 4, 2017

@youknowriad

This comment has been minimized.

Copy link
Contributor

youknowriad commented Sep 6, 2017

Some technical issues here:

  • How to get the duration of an audio file (without some heavy JavaSript)
  • we also need to load JavaScript to the frontend to be able to switch the played track (something we didn't do yet IIRC)

@mtias mtias modified the milestone: Beta 1.2 Sep 6, 2017

@youknowriad youknowriad added the Media label Sep 6, 2017

@mtias mtias added this to the Beta, Needs to happen milestone Sep 6, 2017

@karmatosed karmatosed modified the milestones: 2.4, 2.5, Merge Proposal Mar 7, 2018

@karmatosed karmatosed modified the milestones: Merge Proposal, Merge Proposal: Media Apr 12, 2018

@antpb

This comment has been minimized.

Copy link
Contributor

antpb commented May 17, 2018

Just an update around this issue. I have continued work on this and am in a place where the playlist block is starting to utilize a mediaElement component and pass the appropriate attributes. I hit a bit of a road block recently in getting my code to respect the new changes in 2.3 so I'm finally in a place where everything is caught up. I am now in the process of matching the logic of our current implementation of playlists in core. I apologize on the lack of updates here they will be more frequent over the next week as I work to bring this across the line. Been a bit of a holdup due to my priorities shifting to 4.9.6 readiness :)

@anthonyburchell

This comment has been minimized.

Copy link

anthonyburchell commented May 30, 2018

Another update here: I've chatted with @joemcgill on a different approach for this block and it was decided that we should focus on ID as the main attribute. From there we can get the data we need in an callback render function. I'm experimenting with storing the ID in an element in the the containing div...possibly class names.

I think I'll also follow the approach in the latest-posts block using REST calls to pull the ID's to be handed back to the shortcode render function. (We don't need REST calls since the shortcode is only expecting IDs...) Does this all sound good to you @mtias ? Any missing attributes or bottlenecks I may be hitting here?

@mtias

This comment has been minimized.

Copy link
Contributor

mtias commented Jun 1, 2018

You could store the ID in the comment attr for easiest access. You'll then get in the render callback as a proper attribute.

@antpb

This comment has been minimized.

Copy link
Contributor

antpb commented Jun 3, 2018

Thanks @mtias . Did a bit of cleaning up on my repo and started fresh here: https://github.com/antpb/gutenberg/tree/add/playlist-block-805

hitting publish now saves the ids as we would expect them.

example:

<!-- wp:playlist {"ids":[50,6]} /-->

I'm working on the callback function and I assume this would require core modification to media.php to use the register_block_type function?

register_block_type( 'core/playlist', array(
				'render_callback' => 'wp_playlist_shortcode',
) );

I did that and got undefined function error for register_block_type. Not entirely sure the best way to do this. Going to pick it back up tomorrow. figured I'd leave an update. :)

@antpb

This comment has been minimized.

Copy link
Contributor

antpb commented Jun 3, 2018

Solved the above issues with including the dependencies

I'm currently using the following in media.php and we now have front end loading playlists!

function playlist_block_callback_dependencies() {
    wp_register_script(
        'gutenberg-playlist-block',
        plugins_url( 'gutenberg/core-blocks/playlist/index.js', __FILE__ ),
        array( 'wp-blocks', 'wp-element' )
    );
		register_block_type( 'core/playlist', array(
						'editor_script' => 'playlist_block_callback_dependencies',
						'render_callback' => 'wp_playlist_shortcode',
		) );

}
		add_action( 'init', 'playlist_block_callback_dependencies' );

My question now is, because my implementation is modifying core, is this the right way to go about it? Is there a way to add these dependencies and render callback within Gutenberg instead of media.php?

^ added it to index.php in playlist blocks root directory. Progress!

This is getting very close! Here's a screenshot of the front end:

capture

@themightymo

This comment has been minimized.

Copy link

themightymo commented Aug 15, 2018

+1

@mtias mtias removed this from the Merge: Media milestone Oct 3, 2018

@aaronjorbin aaronjorbin added the Future label Oct 7, 2018

@aaronjorbin

This comment has been minimized.

Copy link
Member

aaronjorbin commented Oct 7, 2018

@anthonyburchell I put the future label on this, but if you are going to have this be a part of the merge can you add it to a milestone and remove the label, please.

@antpb

This comment has been minimized.

Copy link
Contributor

antpb commented Oct 7, 2018

@aaronjorbin yep yep! Agreed we can move to Future.

Just to document here the current status:

The only big blocker in this being included in the merge is setting a condition to include the js templates/playlist dependencies. My PR was including the playlist/video playlist scripts on every page load (in client-assets.php). The new approach takes a similar one to the Code Editor block:

function loadScript() {
return new Promise( ( resolve, reject ) => {
const handles = [ 'wp-codemirror', 'code-editor', 'htmlhint', 'csslint', 'jshint' ];
// Don't load htmlhint-kses unless we need it
if ( window._wpGutenbergCodeEditorSettings.htmlhint.kses ) {
handles.push( 'htmlhint-kses' );
}
const script = document.createElement( 'script' );
script.src = `${ siteURL }/wp-admin/load-scripts.php?load=${ handles.join( ',' ) }`;
script.onload = resolve;
script.onerror = reject;
document.head.appendChild( script );
} );
}
function loadStyle() {
return new Promise( ( resolve, reject ) => {
const handles = [ 'wp-codemirror', 'code-editor' ];
const style = document.createElement( 'link' );
style.rel = 'stylesheet';
style.href = `${ siteURL }/wp-admin/load-styles.php?load=${ handles.join( ',' ) }`;
style.onload = resolve;
style.onerror = reject;
document.head.appendChild( style );
} );
}
let hasAlreadyLoadedAssets = false;
function loadAssets() {
if ( hasAlreadyLoadedAssets ) {
return Promise.resolve();
}
return Promise.all( [ loadScript(), loadStyle() ] ).then( () => {
hasAlreadyLoadedAssets = true;
} );
}

I currently have a mirror of that approach for enqueuing playlist dependencies. I went ahead and updated the PR today to include these new changes:
https://github.com/antpb/gutenberg/tree/add/8050-playlist-refactor-2

There still seems to be an issue where it is not finding wp_underscore_playlist_templates .

After creating a playlist block, console errors: Cannot read property 'replace' of undefined. saving draft/publishing and refreshing the page has no issues. Also publishing and viewing on the front end renders the playlist as expected.

If anyone has any ideas on how to fix this, I'm open to suggestions. Until then, lets leave this on Future. :)

@mtias mtias added this to the Future: 5.1 Onwards milestone Oct 12, 2018

@kiwipaulrob

This comment has been minimized.

Copy link

kiwipaulrob commented Nov 6, 2018

Eager for this feature. I don't have the coding skills to help. Can it be brought forward? 5.1 looks a long way off....

@keramch

This comment has been minimized.

Copy link

keramch commented Dec 18, 2018

migrating existing playlists into a gutenberg-built page is proving problematic... at best overly time consuming to update... i beg you to push this further up the ladder :)

@qantumthemes

This comment has been minimized.

Copy link

qantumthemes commented Jan 8, 2019

It's already about 6 months that people are complaining for missing basic features as playlists.
They shouldn't have forced Gutenberg while still missing basic functionalities that people were already adopting since ages.
It seems a guy with a gun went to Automattic and said YOU PUBLISH GUTENBERG NOW!!!!!!!!!
Like the bad guys from the movies, and then the Automattic developers said "But Sir is still missing..."
And the bad guy said "NOWWW or heads will start blowing!!!"

So we are now forced with an editor that is missing everything.

Only possible reason.

Technically speaking, as keramch says, bringing this to Gutenberg is problematic, as it can be with tons of other frontend custom elements.

This because the Gutenberg principle of taking pieces of the frontend into the editor is wrong per-itself. Page builders add to the frontend editor controls, do not bring the shortcode output IN the backend.

Wrong paradigm, true issues.

Then all goes with placeholders to fix.

@Soean

This comment has been minimized.

Copy link
Member

Soean commented Jan 8, 2019

There is a discussion about deprecating this feature as usage does not seem to be high.
See https://make.wordpress.org/core/2018/12/27/media-meeting-recap-december-21/

@qantumthemes

This comment has been minimized.

Copy link

qantumthemes commented Jan 8, 2019

Thank you Soean for your kind link. The decision is anyway being taken without considering huge niches of the market which are instead relying heavily on this feature in their websites.
But anyway, thanks for the precisation.
Assuming then that this was too hard to bring to gutenberg and being trashed.

Good, less work for us.

@qantumthemes

This comment has been minimized.

Copy link

qantumthemes commented Jan 8, 2019

Also I'd like to ask, what with the thousands of websites already containing playlists in their existing contents? Are they going to disappear or display the raw shortcode?

@keramch

This comment has been minimized.

Copy link

keramch commented Jan 8, 2019

@qantumthemes playlists don't seem to disappear, which is good - they can be managed via classic editor module, but that kind of defeats the purpose, doesn't it? Why wouldn't we all just go BACK to classic editor... which wasn't broken... if we have to use that module to achieve our content requirements inside Gutenberg?
the amazing folks (and I mean that) at Automattic (and frankly, the whole "internet") seem to forget that the VAST MAJORITY of people who are creating websites are individuals and small businesses who just need stuff to work... they don't need (nor care in far too many cases) it to be fancy. They don't want nor have the time/brainspace, to relearn how to do the updates they need to do to their content to share their product, service or information with the world.
If we HAVE to adopt Gutenberg, why not just duplicate a classic editor module and name it "playlist" :)
everyone will be happy and you don't have to spend a huge amount of resources redesigning it.

@kiwipaulrob

This comment has been minimized.

Copy link

kiwipaulrob commented Jan 14, 2019

Is the playlist feature going to make it into 5.1 as previously discussed, or at least into a plug in sometime soon?

@Soean

This comment has been minimized.

Copy link
Member

Soean commented Jan 14, 2019

The WordPress 5.1 beta is out, so we don't add new features to this version.

There were no changes in the PR in the last months: #9169
so I don't think there is something soon.

@karmatosed

This comment has been minimized.

Copy link
Member

karmatosed commented Jan 18, 2019

To me this seems like the type of block could be a custom one in special plugin, over supported in core. I deeply appreciate the work that went into this and would love to see it become a plugin available still.

@gziolo gziolo removed this from To Do in Phase 2 Jan 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment