[wp-trac] [WordPress Trac] #33755: Add Site Logo to WordPress Core

[WordPress Trac]

#33755: Add Site Logo to WordPress Core
-------------------------------------------------+-------------------------
 Reporter:  fatmedia                             |       Owner:
     Type:  feature request                      |      Status:  reopened
 Priority:  normal                               |   Milestone:  4.5
Component:  Customize                            |     Version:  trunk
 Severity:  normal                               |  Resolution:
 Keywords:  ux-feedback has-patch needs-testing  |     Focuses:  ui
  needs-unit-tests has-screenshots               |
-------------------------------------------------+-------------------------

Comment (by celloexpressions):

 In [attachment:33755.14.diff]:
 - Remove the custom customizer control in favor of the core-provided
 `WP_Customize_Media_Control`. A site logo is not a complicated usage of an
 image control. A logo control is a great example of a use case for the
 standard image control that ships with core. In fact, the UI in the
 patches is essentially the same as the core image control. Core customizer
 controls are a demonstration of how to interact with the customizer API,
 and this complex usage is unreasonable for a theme or plugin to consider
 when evaluating the approach to adding their own controls. Themes and
 plugins should be able to accomplish what the previous patches
 accomplished with a custom control with the existing core-provided
 controls, and core should too. We can't launch this feature with the
 custom control class, then go back and remove it later, because customizer
 controls added to the API become available for general use - themes and
 plugins could start using it and core gets stuck with maintaining it. The
 custom control adds a lot of bloat, most notably in the ever-ballooning
 list of CSS selectors for all of the media control styles, for very little
 benefit (also note the significantly reduced patch size, and that even if
 we were to keep a custom control, the implementation could be simplified
 and better future-proofed without functional changes). Due to the time
 crunch, the patch removes the custom control and associated code. I was
 able to roughly implement the custom things (see items below), but someone
 should review as I don't have time to fully test and debug tonight. We
 can't ship with a custom logo control and go back and change it, though,
 so we'll need to live with slightly different button labels and no
 postMessage if it can't be figured out soon.
 - Don't put the logo control at the top of the site identity section. We
 could claim that it's the most important setting, but being firmly theme-
 specific, putting it there would cause the other options to move around,
 which would be disorienting. Additionally, it makes sense to put global
 options before theme options, since they're probably more important to
 set. This area could use further improvement, but placement at the top
 feels like a mistake, much like the former behavior of custom customizer
 sections defaulting to priority 0 (placement at the top). Let's not let
 this hinder existing site-wide settings, especially not in a first pass.
 - The `button_labels` parameter is supposed to be customizable, but I
 messed that up when I wrote the original `WP_Customize_Media_Control` and
 the new image control patches and I guess we never caught it. I added
 those to the control registration, so that element of the custom control
 is preserved (may not fully be working yet though - could use another set
 of eyes). This change is backwards compatible for subclasses that
 previously set button labels by overriding the whole array in a custom
 class constructor - that still works.
 - I added the custom JS that the custom control was doing to the core
 control - I think it could be generally useful. However, I don't have time
 to closely test this or look at any potential side effects so someone
 should review more carefully. If it's not working we should just remove
 the postMessage support for now and revisit later (it's not worth the
 custom control). Situations where the setting is not an id could be
 problematic (instances of `WP_Customize_Image_Control` store a URL).

 Additional things I think should be changed or considered:
 - the `wp-site-logo` body class seems really excessive. Do we want to get
 locked in to having this? There are a lot of body classes already, and I
 don't think there would be common enough usage to warrant it in core; it
 could be added by a theme if there is a specific need for it.
 - There is still a `@todo` in the patch. The comment above it doesn't make
 sense to me, so not sure what's left to do there, but this should be
 resolved (or clarified) before committing to core.
 - We could probably consolidate the site icon control display to show the
 icon next to the browser preview; that would make it take up less vertical
 space if the logo is below.
 - If this is explicitly a theme feature, adding theme support for
 `site_logo` may not the best terminology.
 - Can we add support to Twenty Fifteen? I think at least two bundled
 themes should support this out of the box, possibly more. Fourteen and
 Fifteen could put it at the top of the sidebar.

 Generally, this feels over-engineered to me. However, I don't have time to
 dig in further, so hopefully cleaning up the customizer parts is enough.
 This absolutely should not be committed with the custom control though, it
 dilutes the core objectives of the customizer API and adds a lot of
 duplicated code that isn't needed. Definitely needs some work tomorrow to
 finish cleaning up; I won't be around to help but can circle back at the
 end of the week.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/33755#comment:102>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform