WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#29723 closed enhancement (wontfix)

Media: extract attachment-preview logic from attachment-details-two-column template

Reported by: celloexpressions Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.0
Component: Media Keywords: has-patch 2nd-opinion
Focuses: javascript Cc:

Description

For #21483, we'd like to display media attachments previews in the Customizer controls UI after an attachment has been selected from the media modal. As this applies to both WP_Customize_Upload control and WP_Customize_Image_Control, we would ideally leverage the existing logic that displays either the selected image, an audio/video upload preview, or the appropriate file type icon. Currently this functionality exists for the media library grid view details modal, but it is within a template that also prints the attachment details UI (attachment-details-two-column template).

I'm supplying a basic patch to convey this a bit more clearly, but I'm not familiar with best practices for how this would be implemented in the JS side, so I'll leave formal patching to someone more familiar with this code.

This is a dependency for #21483 now, unless it would be better to just duplicate the logic there... but I'm guessing that's not a great idea.

Attachments (2)

29723.idea.diff (4.8 KB) - added by celloexpressions 6 years ago.
Basic idea, not an actual/functional patch.
29723.diff (4.7 KB) - added by celloexpressions 6 years ago.

Download all attachments as: .zip

Change History (10)

@celloexpressions
6 years ago

Basic idea, not an actual/functional patch.

#1 @wonderboymusic
6 years ago

I can help you with this stuff - I know @ericlewis has been digging into Customizer stuff here at the Times lately, and would be a great asset as well. We should all caucus on Customizer/Media hopes and dreams

#2 follow-up: @wonderboymusic
6 years ago

I think it might be better to make a PHP function that outputs that clump of JS in multiple places

#3 in reply to: ↑ 2 @celloexpressions
6 years ago

Replying to wonderboymusic:

I think it might be better to make a PHP function that outputs that clump of JS in multiple places

That would definitely work for what we need in the Customizer. Where should that live? I have a feeling that we might be able to leverage this in other places in the future as well.

#4 @celloexpressions
6 years ago

  • Keywords has-patch added; needs-patch removed

I think 29723.diff accomplishes this nicely.

This ticket was mentioned in IRC in #wordpress-dev by johnbillion_m. View the logs.


6 years ago

#6 follow-up: @ocean90
6 years ago

  • Keywords 2nd-opinion added

we'd like to display media attachments previews in the Customizer controls UI after an attachment has been selected from the media modal.

Isn't 29723.diff in this case too much? I would like to see #21483 first without the extraction and then review how much code it will share.

#7 in reply to: ↑ 6 @celloexpressions
6 years ago

Replying to ocean90:

Isn't 29723.diff in this case too much? I would like to see #21483 first without the extraction and then review how much code it will share.

Almost all of it is needed, but I can just copy it for now. We're still waiting for #29572 before work can proceed there, though; if you have time to review/commit that that would be a big help.

#8 @celloexpressions
6 years ago

  • Milestone 4.1 deleted
  • Resolution set to wontfix
  • Status changed from new to closed

In the case of the Customizer, the new API for js-templated controls and the specific use-case there work just fine without sharing this code. I still see potential benefits to abstracting this out at some point, but for now we don't need to worry about it.

Note: See TracTickets for help on using tickets.