Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#35969 closed defect (bug) (wontfix)

Twenty Fourteen: Add Site Logo Support

Reported by: celloexpressions's profile celloexpressions Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.6
Component: Bundled Theme Keywords: has-patch has-screenshots
Focuses: ui Cc:

Description

After a bit of thought, I came up with what I think is a fairly elegant way to incorporate a site logo into Twenty Fourteen. This is much nicer than the current approach of shoving a logo into the header image option that I've seen in the wild.

This particular approach can work in addition to or in place of the site title and tagline. If the title and tagline aren't displayed, we may want to consider adding the title into the fixed header as the user scrolls down the page. There are of course other ways to do this, so we should consider some different options before coming to a final decision. The patch doesn't look at handling on mobile yet - it may need to be hidden.

Attachments (4)

35969.diff (1.7 KB) - added by celloexpressions 8 years ago.
First pass.
twentyfourteen-logo-mark-and-text.JPG (150.8 KB) - added by celloexpressions 8 years ago.
Example with logo without text and site title/tagline shown.
twentyfourteen-logo-and-text.JPG (160.8 KB) - added by celloexpressions 8 years ago.
Example logo with text and site title & tagline shown.
twentyfourteen-logo-no-text.JPG (155.7 KB) - added by celloexpressions 8 years ago.
Example with header text hidden.

Download all attachments as: .zip

Change History (14)

@celloexpressions
8 years ago

First pass.

@celloexpressions
8 years ago

Example with logo without text and site title/tagline shown.

@celloexpressions
8 years ago

Example logo with text and site title & tagline shown.

@celloexpressions
8 years ago

Example with header text hidden.

#1 @celloexpressions
8 years ago

  • Keywords has-screenshots added

I attached some examples. Note that the USC ASCE logo is square and intentionally has top padding within the image.

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


8 years ago

#3 @karmatosed
8 years ago

@iamtakashi looping you in here to so we can get your feedback before this goes in.

#4 @iamtakashi
8 years ago

If this is up to me, I'd say not to add support to Twenty Fourteen. The theme obviously wasn't designed for a site logo at that time and there is no good place to add a logo in this theme.

Like Twenty Thirteen doesn't support background color, if the design doesn't make sense with the feature, we don't need to add it just for the sake of it.

#5 @marioi
8 years ago

I got the following error when applying your first pass:

Fatal error: Call to undefined function the_site_logo() in /wp-content/themes/twentyfourteen/sidebar.php on line 18

I like this initiative as most brands require logos on their websites. Twentyfourteen is a theme I'm considering implementing for a client, but if I can't get the logo in, I'll have to install another.

Last edited 8 years ago by marioi (previous) (diff)

#6 follow-up: @celloexpressions
8 years ago

I would say that Twenty Thirteen, for example, definitely wouldn't make sense for a logo. But for Twenty Fourteen, because it's designed to be used for more content-heavy sites, I think a lot of those sites would have logos. I also feel like the top of the left sidebar is an appropriate place for a logo, not sure about mobile. As I said before, a lot of sites are using the custom header for a logo, which doesn't work well at all. Can we give them a better option?

@marioi are you running the latest version of trunk? Sounds like your version might be missing the initial logos commit.

#7 @marioi
8 years ago

Hi @celloexpressions, I haven't done anything with trunk (as a WP dev newbie, I'll research a bit more into this), seems I missed something. It's just a standard WP 4.4.2 installation in which I've added the code (will be using a child theme going forward). I'm fairly new with WP dev but becoming ever more involved with going beyond to tweaking the PHP.

#8 in reply to: ↑ 6 @iamtakashi
8 years ago

Replying to celloexpressions:

In my opinion, the left sidebar isn't a good place for a logo. The sidebar won't be there until 1008px wide viewport and the fact makes a logo need to be hidden for small screens which is bad if you choose to hide your site title in favour of your logo. Of course, there are things to solve the issue with complicated logics but that would simply confuse users.

The only place, if we must to add, is the header where the site title is. If a logo is thin rectangle shape could work but I'd say that's too limiting when we offer an option like this. And it's worth to mention that the sticky header will stop work if the header is more than 48px tall.

I wouldn't argue if the type of sites that uses Twenty Fourteen would or wouldn't likely have a logo. But what I can say is that the theme wasn't designed to have a logo that can be all sorts of sizes and shapes. Therefore I'd suggest to not to add the support in this theme.

#9 @kirasong
8 years ago

  • Keywords close added
  • Milestone changed from 4.5 to Awaiting Review

Per conversation in dev meeting last week, the consensus was to let the original designer decide what themes are appropriate for Theme Logo Support.

As such, as this is @iamtakashi, and he's made clear his decision, suggesting wontfix here.

For context, see the discussion previously linked: https://wordpress.slack.com/archives/core/p1456953047003074

#10 @ocean90
8 years ago

  • Keywords close removed
  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Agreed.

Note: See TracTickets for help on using tickets.