WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#35942 closed defect (bug) (fixed)

Reconsider site logo control placement in customizer

Reported by: celloexpressions Owned by: melchoyce
Milestone: 4.5 Priority: normal
Severity: normal Version: 4.6
Component: Customize Keywords: ux-feedback has-patch has-screenshots commit
Focuses: ui Cc:

Description

Right now, the logo control is at the top of the site identity section, but because this is an option that will be shown and hidden depending on the active theme, this causes the arguably-more-important global site title, tagline, and icon options to move around on different themes, or even when a logo of a different aspect ratio is used. Additionally, we should consider that "site identity" settings general conflict with the notion of a theme-specific setting.

To prevent this new feature from diminishing usage/understanding of the existing core site identity options, two potential options come to mind:

  • Move the site logo control to priority 80, below the site title, tagine, and icon controls.
  • Consolidate the header and background image sections as well as the site logo control to a new images section registered by core (the old sections need to remain registered for back compat, with no controls added to them by default). The header image UI would ideally be cleaned up somewhat first, but this could be a first step to a more unified user expectation of where to find things across themes. This also combats the unfortunate one-control-per-section pattern.

If we decide to keep this in the current location, we do need to revise the priority to something like 10 so that themes and plugins can still put things above it without changing core control positions (in the customizer, no core controls, sections, or panels should be set to priority 0).

Attachments (8)

35942.top.diff (536 bytes) - added by celloexpressions 5 years ago.
Keep the logo control in the same place, but fix the priority so that plugins and themes can place controls above it (no core controls, sections, or panels should have priority 0).
35942.bottom.diff (537 bytes) - added by celloexpressions 5 years ago.
Move the logo control to the bottom of the site identity section, after the global site controls since this is a theme control that isn't always shown.
35942.images-section.diff (4.7 KB) - added by celloexpressions 5 years ago.
Move the site logo, header image, and background image controls to a new unified "images" section, where themes can easily add additional image options. This is probably the best solution, but the header and background image options have fairly extensive UI that should be streamlined (or hidden behind some sore of a toggle to go beyond the basic image control functions) first ideally.
top.png (195.6 KB) - added by obenland 5 years ago.
bottom.png (196.2 KB) - added by obenland 5 years ago.
images-1.png (25.6 KB) - added by obenland 5 years ago.
images-2.png (275.1 KB) - added by obenland 5 years ago.
35942.top.1.diff (573 bytes) - added by obenland 5 years ago.

Download all attachments as: .zip

Change History (27)

@celloexpressions
5 years ago

Keep the logo control in the same place, but fix the priority so that plugins and themes can place controls above it (no core controls, sections, or panels should have priority 0).

@celloexpressions
5 years ago

Move the logo control to the bottom of the site identity section, after the global site controls since this is a theme control that isn't always shown.

@celloexpressions
5 years ago

Move the site logo, header image, and background image controls to a new unified "images" section, where themes can easily add additional image options. This is probably the best solution, but the header and background image options have fairly extensive UI that should be streamlined (or hidden behind some sore of a toggle to go beyond the basic image control functions) first ideally.

#1 @celloexpressions
5 years ago

  • Keywords has-patch added; needs-patch removed

I've attached patches for each of the three options outlined above. Note that we need an adjustment here even if the decision is made to keep it where it is, although I think it would be better to move it to the bottom of site identity for 4.5, and eventually to an images section in a future release.

@obenland
5 years ago

@obenland
5 years ago

@obenland
5 years ago

@obenland
5 years ago

#2 @obenland
5 years ago

  • Keywords has-screenshots added

#3 follow-up: @obenland
5 years ago

Not a big fan of a separate section. I don't feel particularly strong about whether it should be at the top or the bottom, but there have been a good amount of contributors who clearly favor having it at the top (#, #, #).

At this point I'm inclined to just go with 35942.top.diff.

#4 @celloexpressions
5 years ago

Also worth noting that #35943 would reduce the amount of vertical space (and visual noise) taken by the site icon control, which could mitigate concerns about site logo becoming hidden if it's at the bottom. We may want to reconsider after that's completed. Should also seek new feedback from everyone now that this has been redefined as a strictly theme-specific feature that will not be enabled by core by default, meaning that putting it at the top would cause the other controls to jump around between themes. That behavior should probably be user-tested.

This ticket was mentioned in Slack in #core by adamsilverstein. View the logs.


5 years ago

#6 @webifish
5 years ago

I vote for top position and not a separate section too.

This ticket was mentioned in Slack in #core by obenland. View the logs.


5 years ago

#8 @mikeschroder
5 years ago

  • Owner set to melchoyce
  • Status changed from new to assigned

Assigning to @melchoyce for UX review/approval.

@melchoyce, feel free to reassign after approval if you'd prefer not to handle the commit.

#9 in reply to: ↑ 3 ; follow-up: @melchoyce
5 years ago

Replying to obenland:

At this point I'm inclined to just go with 35942.top.diff.

Same, but I'm having trouble applying the patch — getting Hunk #1 FAILED at 1981 -- saving rejects to file src/wp-includes/class-wp-customize-manager.php.rej

#10 in reply to: ↑ 7 ; follow-up: @helen
5 years ago

Replying to slackbot:

This ticket was mentioned in Slack in #core by obenland. View the logs.

Realized that we had a long discussion and I didn't summarize here. Not sure if anybody is actually tackling this at this point, but in short I think the biggest problem is that having multiple image controls visible at once is extremely overwhelming and we should at least make some kind of overture at how that could be addressed. The placement within that panel almost doesn't really matter at the moment because it's a lot to process visually in any order. I think there are some pretty solid ideas for changes to image controls to make them more compact, including #35943, but recognize that it would be a broader change than one would hope to be making at this point.

Excerpting part of the conversation here for easier reference:

helen: Sure. I still think image controls look pretty wild and are maybe even overly large for something you are allegedly live previewing.

melchoyce: I think we can definitely simplify
Do we need full buttons for “remove” and “change image"

helen: Do they really need to be so big when it’s being displayed in context anyway?

melchoyce: Also true
Though horizontal logos at least won’t look so big

helen: Full buttons: possibly not? That’s perhaps a change that needs testing done.

melchoyce: Yeah, I’d want to redesign image controls totally independently of this
I think an X suffices for remove (like when you add a header image, and your old ones show up below with X to remove)
And my gut reaction is to always click an image to replace or edit it

helen: Sure, and we have the pencil/X paradigm in the editor already, yeah?

celloexpressions: I agree on the x and click-to-edit (edited)
header images also use the X to remove in the customizer

helen: Maybe not really a controversial change
Anyway, I know that we’d rather think about it independently, etc., but I’m not sure about shipping this in an overwhelming state.

@obenland
5 years ago

#11 in reply to: ↑ 9 @obenland
5 years ago

Replying to melchoyce:

Same, but I'm having trouble applying the patch

Refreshed the patch in 35942.top.1.diff. It doesn't change anything visually but allows other controls to be added earlier.

#12 in reply to: ↑ 10 @celloexpressions
5 years ago

Replying to helen:

I just opened #36175 to track this. Also related: #35943. However, I still prefer placing the logo control lower regardless of how much space it takes up, for the reasons I outlined previously.

#13 follow-up: @melchoyce
5 years ago

Got this working with Twenty Sixteen and I'm noticing the logo doesn't appear retina in the sidebar (though it displays correctly on the site itself). As far as I can tell, this isn't an issue with header images. Is this a quick fix?

Retina logo aside, everything else looks good to me. I'm still in favor of the logo at the top.

#14 in reply to: ↑ 13 ; follow-up: @celloexpressions
5 years ago

Replying to melchoyce:

Got this working with Twenty Sixteen and I'm noticing the logo doesn't appear retina in the sidebar (though it displays correctly on the site itself). As far as I can tell, this isn't an issue with header images. Is this a quick fix?

Retina logo aside, everything else looks good to me. I'm still in favor of the logo at the top.

That would be a new ticket, which would be for the media control in general. We could load larger images by default, or do srcset, but there are other implications there with load time.

#15 in reply to: ↑ 14 ; follow-up: @melchoyce
5 years ago

  • Keywords commit added

Replying to celloexpressions:

That would be a new ticket, which would be for the media control in general. We could load larger images by default, or do srcset, but there are other implications there with load time.

Cool, can make a new ticket for that. I think this is good to commit.

Do you know why header seems to be retina by default? Does it just load a larger image?

This ticket was mentioned in Slack in #core by obenland. View the logs.


5 years ago

#17 @obenland
5 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 36912:

Customize: Bump down the priority custom logo's control.

Keeps the control in the same place but allows for plugins and themes to place
other controls above it.

Props celloexpressions.
Fixes #35942.

#18 in reply to: ↑ 15 @celloexpressions
5 years ago

Replying to melchoyce:

Do you know why header seems to be retina by default? Does it just load a larger image?

Probably? We should probably go through all of the places that images show up in the customizer and make sure they account for hidpi (thinking it's primarily just image controls and site icon).

#19 @melchoyce
5 years ago

Reported the logo issue in #36191.

Note: See TracTickets for help on using tickets.