Refactor Customizer Upload, Image, and Background Image controls to leverage the media library/modal

[jjonesftw]

I suggest that users be able to use any image in the media library as a background image from the customizer. At this time, if a user changes the theme background from the customizer, he/she must upload the image. This is already available from the dashboard, under Appearance > Background - it gives the option "Or choose an image from your media library:" I suggest that this option available on the dashboard also be available on the customizer. A similar option should be available for header images.

[sabreuse]

#24510 was marked as a duplicate.

[prionkor]

+1

[shelob9]

CC me@ added.

Also big +1

[celloexpressions]

+1

Once we do this, we can do #25569 and drop the separate background page entirely.

[celloexpressions]

I believe this might actually be very straightforward. Once #25618 happens, we'd just need to adjust the WP_Upload_Image control to open the media modal (which has upload functionality) instead of opening the browser's upload functionality directly, just like we do in several places in core and as plugins do all over the place.

In fact, the best solution is probably to make WP_Customize_Upload_Control open the media modal (as it provides the best experience for uploading and browsing files of any type; I'd say leave the dropzone too for quick access), then we can just make the necessary adjustments to WP_Customize_Image_Control and WP_Customize_Background_Image_Control. That way all instances and extensions of upload-type controls get the best experience we have.

[celloexpressions]

#25618 happened, so now it's just a matter of replacing the upload button with the media modal, where you can select or upload.

We can re-brand the "Upload Image" and "Uploaded" tabs to make the library of uploaded images be for quick access to other images used as header images. But we could also replace the whole interface with a media library button, which may be a more elegant solution. Thoughts?

[prionkor]

Just using the media library might be the better idea. No need for remembering previously uploaded images we just choose any images from medial library.

[Kniebremser]

Or another idea:

• In media upload folder create a new folder backgrounds.
• If I want to upload a picture, I can mark this as a background picture.
• This picture is in the folder "backgrounds" stored only in original size without resizing.
• And the pictures are me to choose from in Costumizer -> themes -> background pictures available.

[ocean90]

I think we should try to do #21785 for background images. Without cropping, just the UI and the media uploader.

Related: #22148

[helen]

#28491 was marked as a duplicate.

[nacin]

#27454 was marked as a duplicate.

[celloexpressions]

I think we're all in agreement that backgrounds should be handled similarly to headers (#21785), without cropping, and that we can fix up the generic Upload and Image controls in the process.

Unlike with headers (which have the randomize option), these controls don't need to support multiple files, so we could potentially remove the concept of showing images that were previously uploaded to this control. Using the media modal (added in 3.5) makes it easy to find already-uploaded files, and we could add "uploaded to this control" into the modal's filtering options.

[celloexpressions]

First-pass, slightly more than a proof-of-concept. JS portions partially based on work from @mattwiebe. Needs back-compat for subclasses, UI for previewing media in the generic upload control, i18n, coding standards/cleanup, docs, and UI/UX feedback.

[celloexpressions]

UI walk-through with WP_Customize_Upload, WP_Customize_Image, and background image controls with 21483.diff​.

[celloexpressions]

Let's do this.

21483.diff​ contains a rough first-pass at re-imagining the core media controls to leverage the media library. Some notes:

• The JS is based off of @mattwiebe's ​Advanced Upload Control project, although I've started making extensive changes.
• The goal for this ticket is to make these core controls as good as they can be in a backwards-compatible way. This means that, unfortunately, we cannot change what data is saved in the settings associated with these controls. Therefore, we have to stick with only grabbing the media file's URL from the media library. We'll do more exciting things in #29215.
• For now, I've removed the concept of previously-uploaded files; with access to the media library, and because we don't have "multiple" support (like header images do, with the randomize option), the extra UI doesn't seem necessary at this point. The exception may be background images, although I'd argue that it isn't necessary, as they're accessible and searchable in the library.
• Don't waste screen real estate for a small file-upload dropzone; while opening the library first is an extra step, there's a full-screen dropzone as soon as it's open.
• Still need to implement a strategy for the default background image (and add handling for any of these controls that have default setting values). There's a lot of theme inconsistency here, as many (most?) themes that support custom backgrounds also set the background in CSS, making it impossible to remove the default background. Maybe remove should just be "default" for those? I tried checking what the backgrounds admin page does but, well, it's just a complete mess.
• There's an awesome amount of red happening here. Unfortunately, most of it will probably need to be brought back for back-compat. Initial patch does that to demonstrate the advantages of leveraging the new code; it's really fairly straightforward. The biggest back-compat issues are with sub-classes, and there are a lot of these in the wild (particularly with WP_Customize_Image_Control). We'll need to get creative here.

Things that the patch still needs to address:

• How should generic-upload-control files be previewed in the Customizer controls area? There's probably something we can leverage from elsewhere in core. This would be things like audio/video, images, or other document types that need to be linked but can get an icon. Note that we can't display the document's title since we can only save the URL for back-compat reasons, unless we set it in the control from JS when inserting, and use something like attachment_url_to_post_id() to grab the info in PHP on subsequent loads.
• Handling of default setting values when "remove" is invoked, and whether to allow removing the default (setting to empty), or to just make it a "default" button instead of remove.
• Consider making the button/media manager labels customizable parameters of the WP_Customize_Upload_Control class, which would also further reduce the need for duplicated code in subclasses.
• JS Strings need to be internationalized.
• Coding standards.
• Structured inline docs.
• Potential code cleanup, especially in JS.
• Comprehensive back-compat audit. It's likely that we won't be able to mimic the "extensions" argument easily, but the media library's mime_type feature is much more useful for most cases anyway. We need to keep all of the class functions around, and just remove any calls to them in the least obtrusive way possible so that we can avoid breaking anything sub-classing these (in PHP and JS). I've removed all of these for now - we'll add them back in once we figure out the strategy here. We probably need to go beyond just avoiding fatal errors in subclasses to making things that used to work still work - an initial investigation revealed a LOT of subclasses of WP_Customize_Image_Control in the wild. We need to investigate this more thuroughly to determine the best approach here, as those would ideally also be able to benefit from the new improvements. Again, the goal here is to bring a better experience to all of the existing controls that are out there; it'll be challenging but I think we can do it.
• Go through all media-related tickets in the Customize component, and determine whether they're still relevant once this change is made.

[celloexpressions]

For control-previewing we should leverage the logic that's used in the media library grid view details modal. That's currently housed within a JS template that also does the UI for the other fields, so I've created #29723 to extract it to facilitate re-use. If anyone's well-versed in the media library's (Backbone) architecture, please take a formal stab at that.

Once that's in place, we should be able to leverage attachment_url_to_post_id() to locate the necessary data for this template when re-opening the Customizer to display an existing attachment. We can of course just pass it over from the media modal when selecting a new file. The biggest restriction with all of this is that we can only save the file URL as the control's setting's value.

I'd also like to fully leverage #29572, so I'm going to hold off on continued iterations here until those two things are in core. Anyone following along, please test/patch/leave feedback on/help move along #29723 and #29572 so that we can get back to this sooner rather than later.

[dauidus]

This seems to have been implemented ​here, although a bit differently. I do believe it is backwards-compatible, though.

EDIT*
Found a better solution than what I posted above. ​Extend WP_Customize_Image_Control

This implementation is NOT backwards compatible, at the moment. It does, however, do exactly what was originally proposed by @jjonesftw and what was implemented by @celloexpressions. Please forgive me if I'm beating a dead horse here - just trying to explore all options. The solution I have suggested does provide the best UI I've seen to date, when it comes to uploading images in the Customizer.

[celloexpressions]

Replying to dauidus:
I think we should ditch the tabs if at all possible - leads to a cluttered/confusing UI and probably unnecessary. We're also making it consistent with the generic upload control, which currently doesn't have tabs. Do you see a particular reason for keeping the tabs/adding them to the upload control?

Implementation-wise, we should take advantage of the opportunity to rewrite this directly rather then hooking in, as a theme or plugin would do. At the same time, this should make it easier for plugins and themes to hook in, given the updated, js-based code structure.

There are several plugin/theme implementations out there (including the one from @mattwiebe that the patch is based on), although any would require reworking to be ready for core.

[dauidus]

Yes, I agree that the tabs should be completely lost. Their inclusion adds one extra click to get where you want to be, not to mention they add to a potentially overloaded customizer system already (assuming that some devs won't respect any sort of section limit. Well, it really adds an extra 2 clicks... assuming we use the default add media panel - all options are in there, easily sorted out for quick overview of all possible upload decisions.

And, I do agree that it should be easy for future devs to hook into. You're spot-on there.

I'm looking through all threads on this now, and getting caught up more before I speak to this any more... I didn't realize we were already moving along on this as far as we have.

Cheers!

[celloexpressions]

#29770 was marked as a duplicate.

[celloexpressions]

This ticket is still waiting on #29723 and #29572 to be committed, or at least to receive some dev feedback so that revisions can be made and then they can be committed. Those really need to happen ASAP for this to get into 4.1, which it really should, especially now that we can see how much better of an experience that offers with Twenty Fifteen's use of this feature. Timeline-wise, it's going to be tight but currently still doable, and bumping back beta or making this a task and continuing additional work here into the first beta would also help. I can't really do much more here until those other tickets are completed, but once they are I have a pretty good idea of how to leverage the current patch here and those new additions to resolve all of the outstanding issues listed in 22.

[celloexpressions]

Rough first pass with the new js-templated-controls framework. Implements previews of all media types, upload and image controls, and basic attempt and reasonable back-compat. Background Image control not yet updated to demonstrate back-compat with sub-classes of WP_Customize_Image_Control. Previewing of existing (saved) control setting values not yet implemented.

[celloexpressions]

Implement display of existing (saved) control settings, via attachment_url_to_postid().

[celloexpressions]

This now works pretty well, and the new JS templating API lends itself especially well to this. The slightly modified media code allows us to offer some visual preview/feedback for all file types, basically for free. It's time for UX, dev, and UI feedback - please test the patch, review the code, evaluate the new workflows, consider back-compat, etc. Patch iterations are welcomed.

Known issues with 21483.2.diff​:

• Need to address handling of the default setting value. Would probably be a "default" link in place of remove, then switching to remove when the default is being used. But we can't easily preview the defaults, unfortunately, as they aren't usually attachments, so we'll probably implement that for images only.
• The labels need to be translated, but PHP doesn't seem to like doing that the way I've implemented it. How can we do that, keeping in mind that the labels should be able to be overridden as easily as possible.
• Audio and video previewing basically work (yay), but there seems to be something missing that actually initializes MediaElement.js once the elements show up in the DOM. Perhaps @wonderboymusic has thoughts?
• Should there be some sort of core refresh/reRender functionality, or is it best to leave it to specific controls? How's the implementation here @westonruter?
• Back-compat: the background image control is left unchanged to demonstrate that subclasses of WP_Customize_Image_Control (and by extension WP_Customize_Upload_Control) generally don't experience significant breakage - we'll truncate it like WP_Customize_Image_Control once it's looking good.
• This does deprecate some things and quietly ignore some things (like WP_Customize_Upload_Control::$extensions). The upside clearly outweighs those here, but are there any particular things that we could do to make the transition smoother?
• Could use some accessibility feedback on this. I did a quick keyboard accessibility check, and while the media modal part is just as bad as it is everywhere else in core, it is functional. Needs screenreader testing for sure, and not sure how the <label> should be placed.
• Could probably have more structured inline docs, but not sure exactly what's needed, @DrewAPicture?
• For backgrounds, will we want to keep the concept of previously-uploaded backgrounds, like headers have? That would introduce more complexity, so we might want to wait until we do things like adding support for multiple (or randomized) backgrounds.

General note: we'll be able to close a bunch of tickets once this gets in, since bugs ranging from no error messages to favicon uploads failing no longer exist.

[celloexpressions]

Test plugin with an upload control and an image control, spitting the values out into the preview.

[celloexpressions]

This patch needs to be refreshed for #28709 / [30102]. Due to no feedback, I'll also do my best to address as many of the above points with assumptions that will work for a first pass and can be cleaned up further during beta.

[celloexpressions]

Refresh, plus some additional refinements. Default handling partially implemented.

[ocean90]

21483.3.diff​:

• Can we use the constructor to set up $button_labels?
• Button labels for background images should include "image" not "file"
• wp.customize.ImageControl needs to stay for BC. Why does it need to be removed? On PHP side we still have WP_Customize_Image_Control extends WP_Customize_Upload_Control. I think it's OK to have ImageControl to just handle images.
• src/wp-admin/js/customize-controls.js:894 has a missing semicolon
• Can we have a "No image set" hint for the background control too?
• Drag&Drop: Let the whole control be a dropzone, that would be awesome. After the drop the frame should open for upload status and error reporting

[celloexpressions]

No-media placeholders, and general UI layout.

[celloexpressions]

Iterations per @ocean90's feedback.

[celloexpressions]

Replying to ocean90:

21483.3.diff​:

• Can we use the constructor to set up $button_labels?

Yes, I'm afraid that's going to be the only option. Not as easy to override in subclasses or individual instances, but works fine.

• Button labels for background images should include "image" not "file"

Should be fixed in .4, that control was not yet changed at all - that is what the back-compat looks like currently for a subclass of WP_Customize_Image_Control.

• wp.customize.ImageControl needs to stay for BC. Why does it need to be removed? On PHP side we still have WP_Customize_Image_Control extends WP_Customize_Upload_Control. I think it's OK to have ImageControl to just handle images.

None of the JS is needed at all due to the way the media modal works - and all of the image logic needs to be in the upload control anyway since it supports images. But we can leave the wp.customize.ImageControl model, empty its methods, and remove it from the control constructor declaration so that subclasses should still work as expected. Can this look essentially like how the PHP class looks in the latest patch?

• src/wp-admin/js/customize-controls.js:894 has a missing semicolon

Nice catch!

• Can we have a "No image set" hint for the background control too?

Done.

• Drag&Drop: Let the whole control be a dropzone, that would be awesome. After the drop the frame should open for upload status and error reporting

I'm not sure that this is needed, or particularly discoverable. Also, not a very big space for a dropzone. But we could add it if people want it.

[celloexpressions]

Remaining todos: debug default handling, docs audit, audio/video init issues.

[jakilevy]

@celloexpressions - If this is meant to work in the customizer, and the customizer is customizing background images and headers, why do we need audio/video to work in this upcoming release?

[celloexpressions]

Clarification on everywhere that this impacts:

• All media-related controls that theme and plugin developers implement via the Customizer API; specifically, instances and subclasses of WP_Customize_Upload_Control (and by extension WP_Customize_Image_Control. This technically supports all filetypes that are allowed to be uploaded, and will be an improvement for all of them. Audio/video is included, and we can incorporate audio/video previewing with minimal effort.
• Core background image control.
• NOT the header image control - eventually that should be adjusted to use some of the newer APIs that we've developed here.

[ericlewis]

wp.customize.UploadControl.rerender() can be dropped in lieu of clearing out the container ( .html('') ) in wp.customize.Control.render, and using event delegation on the control ($container.on( 'click', '.thing' ... )) rather than binding directly to the elements.

[ericlewis]

attachment:21483.6.diff​ adds some more documentation to attachment:21483.5.diff​.

Eventually we can add doc blocks for the subclasses of api.Control, but fully fleshed out documentation for the parent class api.Control should probably get in place first, but can happen separate from this effort.

[celloexpressions]

Thanks @ericlewis! We're pretty close to being ready for a first-pass commit; additional issues can be addressed during beta.

[celloexpressions]

Remove reRender, other related cleanup

[celloexpressions]

Implement default handling. Still some issues around trying to remove default images, although many themes don't support this anyway.

[celloexpressions]

I think 21483.8.diff​ is good enough for a first-pass commit. It addresses all remaining issues here other than the wonkiness with audio/video, which wasn't supported previously anyway.

[ocean90]

In 30309:

Customizer: Add support for the media library in Background Image controls.

WP_Customize_Upload_Control (PHP) and api.UploadControl (JS) can now open the media modal.

props celloexpressions, and ericlewis for docs.
see #21483.

[ocean90]

I see a "Default" Link in the Background Image control, but it seems like it does nothing.

[celloexpressions]

fix backwards conditional

[ocean90]

In 30553:

Customizer: Only show the default button in the upload control if there is a default attachment.

props celloexpressions.
see #21483.

[ocean90]

21483.10.diff​:

• Makes the UI consistent with Custom Header
• Removes audio/vide preview which doesn't work yet, we can improve this later
• Removes "Remove" button for the default value
• Avoids calculating of the container/placeholder height unrelated to custom headers

[celloexpressions]

I'm okay with 21483.10.diff​, but I have the following mostly design-related concerns:

• Can we remove the border around the preview if it's not empty? Looks pretty bad for non-images, as there's no padding around the icon, and it just seems unnecessary.
• Also not liking the 2px border radius on images and the container. If we want to be consistent I'd remove that for headers too.
• Could we possibly have fewer container divs around the previews? Looks like there are four levels of nested divs, and we should remove at least div.current and div.container since they're meaningless name-wise. We don't need all of that CSS here either - we should not set a min-height for these, for example. Maybe start by having fewer containers, then clean up unneeded or less attractive styles. We can tweak the headers UI design too, and I think it could definitely use updating here.
• I really don't think default/remove should have the same visual hierarchy as change/select. A button and a link would make way more sense than two buttons, although I don't have a preference for left vs. right aligned (and this goes for headers too).

The functional bits here are fine. To clarify for anyone following along, audio/video are still supported, they just won't have interactive previews (yet).

[ocean90]

In 30712:

Customizer: Enhance Background Image/Upload controls.

• Adopt the Header Image layout to the Background Image/Upload control.
• Remove templates for interactive audio/video previews. They haven't worked yet.
• Don't show a "Remove" button when the default value is set.
• Avoid calculating of container/placeholder heights which are unrelated to custom headers.

see #21483.

[ocean90]

Calling this as fixed for now.

@celloexpressions: I used the same containers/button types because I didn't want to implement something new because it's really late in current cycle. That was the simplest solution.

Enhancements/issues should go into new tickets.

[wonderboymusic]

In 30737:

Show title, album, and artist info for audio files displayed in WP_Customize_Upload_Control().

See #21483.

[nathanrice]

Replying to ocean90:

Don't show a "Remove" button when the default value is set.

Why not? Why is there not a remove button anywhere? This is definitely a feature that users use. There needs to be a way for a person to remove a background image if they want to.

[dreamwhisper]

Replying to ocean90:

• Don't show a "Remove" button when the default value is set.

This change is not backwards compatible with themes have extended the image upload and coded to account for Default, Upload, and Remove functionality. Instead, clicking default removes the image from the front end output.

[ocean90]

Replying to dreamwhisper:

This change is not backwards compatible with themes have extended the image upload and coded to account for Default, Upload, and Remove functionality. Instead, clicking default removes the image from the front end output.

Please open a new ticket with a small plugin to demonstrate the issue.

[ocean90]

In 30849:

Customizer: Re-add the possibility to remove and restore a default background image.

see #21483.
fixes #30683 for trunk.