WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 2 months ago

#32332 closed defect (bug) (fixed)

Twenty Fifteen: Add Missing Social Link Genericons

Reported by: philiparthurmoore Owned by: lancewillett
Milestone: 4.3 Priority: normal
Severity: normal Version: 4.3
Component: Bundled Theme Keywords: has-patch
Focuses: Cc:

Description

Skype and Path are missing. See screenshots and attached patch.

Attachments (4)

32332.diff (519 bytes) - added by philiparthurmoore 4 months ago.
before-patch.png (43.4 KB) - added by philiparthurmoore 4 months ago.
after-patch.png (39.3 KB) - added by philiparthurmoore 4 months ago.
32332.2.diff (519 bytes) - added by philiparthurmoore 4 months ago.

Download all attachments as: .zip

Change History (16)

@philiparthurmoore4 months ago

comment:1 @lancewillett4 months ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.3

Thanks for this, Philip.

comment:2 follow-up: @lancewillett4 months ago

@philiparthurmoore I think the CSS rule should use asterisk (*) instead of dollar sign ($) for those two additions.

comment:3 in reply to: ↑ 2 @philiparthurmoore4 months ago

Replying to lancewillett:

@philiparthurmoore I think the CSS rule should use asterisk (*) instead of dollar sign ($) for those two additions.

Gah. You're right. New patch coming shortly.

@philiparthurmoore4 months ago

comment:4 @lancewillett4 months ago

  • Owner set to lancewillett
  • Resolution set to fixed
  • Status changed from new to closed

In 32497:

Twenty Fifteen: add missing social link Genericons styles for Skype and Path.

Fixes #32332, props philiparthurmoore.

comment:5 @iamtakashi4 months ago

Just FYI, we removed Skype intentionally. See #30059.

comment:6 @iamtakashi4 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I don't use Path so I don't know much about it, but is there each user's public facing page with a url in Path?

Last edited 4 months ago by iamtakashi (previous) (diff)

comment:7 follow-up: @philiparthurmoore4 months ago

Thanks for the comments, Takashi. My thinking behind this was that the social icons are available in Genericons, and the only two missing from Twenty Fifteen, so that seemed like an oversight. I totally missed #30059. Does every single social network that's supported in Twenty Fifteen have (or need to have) a public profile URL? I thought that these social icons were supposed to have parity with Genericons first, and then take into consideration the public profile URL next, so didn't think about whether or not a public URL exists for the icons, when path.com and skype.com links would suffice as well. Why would someone want to put an empty Path or Skype top level URL into their social menu? Not sure, but the missing icons felt weird. :) Thanks for referencing that older ticket.

Last edited 4 months ago by philiparthurmoore (previous) (diff)

comment:8 in reply to: ↑ 7 @philiparthurmoore4 months ago

Replying to philiparthurmoore:

Does every single social network that's supported in Twenty Fifteen have (or need to have) a public profile URL?

For example, polldaddy.com comes to mind. I'm sure there are others. Probably much too late to get into auditing those services for public profile URLs?

comment:9 @iamtakashi4 months ago

I can't tell with a confident if all supported services have public facing user's pages but I couldn't find it straight away in Path, that's why I didn't add the support, and I removed Skype for the reason in the ticket.

But, if we really think the support should depend on what icons available in Genericons, I don't mind adding the support for those services.

Does anyone have an opinion on this?

comment:10 @obenland2 months ago

If you can't link to those social networks we probably don't need to support them.

comment:11 @philiparthurmoore2 months ago

I'm fine with either approach. So maybe revert this fix, and in future themes that support social networks make sure they have public-facing links?

comment:12 @lancewillett2 months ago

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

I'm happy to leave it as-is. Skype links technically work—and someone might want to have that protocol in their menu.

Note: See TracTickets for help on using tickets.