Opened 7 years ago
Closed 7 years ago
#42495 closed defect (bug) (fixed)
Customizer: Widget-embedded playlist: Incomplete preview
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.9.3 | Priority: | normal |
Severity: | normal | Version: | 4.9 |
Component: | Customize | Keywords: | has-patch commit fixed-major |
Focuses: | Cc: |
Description
When adding a playlist to a text widget via the playlist
shortcode, the preview simply shows a white box where the playlist will be. After publishing and reloading the page, the preview renders a full preview of the playlist, but any changes to the text widget cause the playlist preview to revert to the white box.
Attachments (5)
Change History (30)
#1
@
7 years ago
- Keywords needs-patch added
- Milestone changed from Awaiting Review to 4.9.1
Good catch. We need to unconditionally enqueue the wp-playlist
JS statically or lazy-load it when selective refresh happens when playlists are used.
For the static route, we'd need to introduce the enqueue_preview_scripts
logic from the media widgets:
https://github.com/WordPress/wordpress-develop/blob/c994a2e/src/wp-includes/widgets/class-wp-widget-media.php#L109-L111
https://github.com/WordPress/wordpress-develop/blob/c994a2e/src/wp-includes/widgets/class-wp-widget-media-video.php#L159-L176
For the dynamic route, it would require more work. We'd need to sniff out whether a playlist is included in the rendered partial and then dynamically add the required JS and CSS. This is done in Jetpack for infinite scroll. I don't think we'd need to go the dynamic route, however.
#2
@
7 years ago
We'd probably also need to hook in the logic to set up the playlists when the partial-content-rendered
event happens. See
https://github.com/WordPress/wordpress-develop/blob/c994a2e9bb094e51506e600aa1fc3862c63a934c/src/wp-includes/js/customize-selective-refresh.js#L482-L489
See also Jetpack PR: https://github.com/Automattic/jetpack/pull/3543/files
#5
@
7 years ago
@obenland Yeah, I think they're a good place for me to start. I'm going to take the opportunity to understand more about how preview works.
#6
@
7 years ago
- Keywords has-patch added; needs-patch removed
I created a PR for this because it has been a good way to collaborate with @westonruter in the past:
https://github.com/xwp/wordpress-develop/pull/303
I'm also attaching a patch with current progress.
#7
@
7 years ago
Note that this patch doesn't conditionally enqueue playlist scripts based on a filter like we do with wp_video_shortcode_library
because AFAICT no similar filter currently exists for the playlist shortcode.
This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.
7 years ago
#9
@
7 years ago
- Keywords needs-testing added
- Owner set to westonruter
- Status changed from new to reviewing
@bpayton anything left here as far as you know?
This ticket was mentioned in Slack in #core by desrosj. View the logs.
7 years ago
#13
@
7 years ago
Looks like the patch might need a refresh:
Hunk #1 FAILED at 3. Hunk #2 succeeded at 166 (offset -4 lines). 1 out of 2 hunks FAILED -- saving rejects to file src/wp-includes/js/mediaelement/wp-playlist.js.rej patching file src/wp-includes/widgets/class-wp-widget-text.php Hunk #2 succeeded at 393 (offset -4 lines). Hunk #3 succeeded at 414 (offset -4 lines). patching file src/wp-includes/widgets/class-wp-widget-text.php Hunk #1 succeeded at 403 (offset -4 lines). Hunk #2 succeeded at 416 (offset -4 lines).
#16
@
7 years ago
- Keywords needs-refresh removed
@afercia let me know that patches can be generated in a better way for svn
. Does this help?
#17
@
7 years ago
@bpayton Git is OK to generate patches and as @jorbin mentioned on Slack, grunt patch
takes care of hte format.
#18
@
7 years ago
I was glad to hear about grunt patch
. It's helpful to avoid these kinds of issues. The patch was just an easy thing to update. Generating an SVN-friendly patch is about as easy as generating and unfriendly one., so I figured why not? Maybe a reviewer doesn't have grunt
and JS deps set up and doesn't want to think about it right now.
Thanks again for your help today.
This ticket was mentioned in Slack in #core by desrosj. View the logs.
7 years ago
This ticket was mentioned in Slack in #core-customize by westonruter. View the logs.
7 years ago
#22
@
7 years ago
- Keywords commit fixed-major added; needs-testing removed
- Resolution fixed deleted
- Status changed from closed to reopened
Incomplete playlist preview