[wp-trac] [WordPress Trac] #16434: Give site admin ability to upload favicon in Settings, General

[WordPress Trac]

#16434: Give site admin ability to upload favicon in Settings, General
--------------------------+------------------------------------------------
 Reporter:  jane          |       Owner:  obenland
     Type:  task          |      Status:  accepted
  (blessed)               |   Milestone:  4.3
 Priority:  normal        |     Version:  3.1
Component:  Customize     |  Resolution:
 Severity:  normal        |     Focuses:  ui, accessibility, administration
 Keywords:  ux-feedback   |
  has-patch               |
--------------------------+------------------------------------------------

Comment (by celloexpressions):

 Thanks for .16.diff. It looks pretty good to me too, a few additional
 notes:
 - `apply_filters( 'wp_ajax_cropped_attachment_id', 0, $context );` seems
 like it should be an action, not a filter, based on the core usage for the
 site icon case. Additionally, I think this is a case where it may make
 more sense to have a dynamic hook instead of passing the context as a
 param. Are there situations where you'd want to filter multiple contexts
 together? I think it would be most common to filter/add an action here to
 add custom handling for your custom context, in which case a dynamic hook
 would probably read better (thinking along the lines of some of the ones
 for types in `WP_Customize_Setting`.
 - It also looks like anyone wanting to use the cropped image control would
 have to implement their own handling for saving the attachment when the
 image is cropped. In the Customizer at least, this should "just work".
 Default behavior should probably be to save a copy of the attachment and
 return the new attachment. In most cases it won't be necessary to change
 that behavior when setting the
 - The customizer media control is designed to use the full image url, not
 the medium size, when rendering the image preview, so we should probably
 pass the full (cropped, likely 512px) image as the setting's default
 value. Note that the image can be displayed as wide as ~ 560px on certain
 screen sizes.
 - I like the idea of using the control type as the context, but I'm not
 sure that it's going to be better for developers in practice. I have a
 feeling there will be scenarios where you may need to create a custom
 control just to change the context, where no other changes are necessary.
 That has major implications in terms of effort required (registering the
 new type, defining it in PHP and JS, duplicating all of that nasty core
 CSS just to use the core UI, etc.), whereas being able to set it as a
 parameter would be fairly straightforward. This is also designed to be
 used as the `context` of the attachment once it's created by default,
 which could potentially allow this control to offer an option to show a
 library of uploaded/cropped images in the future (like header images).
 - "Site Identity" has been my thought for this for some time and I think
 it works quite well. Would like to hear any other opinions on that/discuss
 it further but that can potentially happen after a first pass. The concept
 for this section is/becomes things that identify the site outside of the
 actual site (ex. in the browser icon/title), and can also show up on the
 site depending on the theme.

 Should be pretty easy to add a check for whether the image is exactly the
 requested size in `wp.customize.CroppedImageControl.onSelect()`, calling
 `onSkippedCrop()` instead of switching to the cropper state if it's the
 right size. I'd only do that if flex-height and flew-width are disabled.

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