Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update: Social Logo to the latest version. #2950

Merged
merged 1 commit into from
Feb 2, 2016

Conversation

enejb
Copy link
Member

@enejb enejb commented Feb 1, 2016

Added hover styling on the dev/docs
Added Share Icon
Update twitter logo
Added twitter-alt logo.

The reason why the twitter-alt is more for consistency.
Most of the services have a box around the logo.

After:
screen_shot_2016-02-01_at_11_33_26

cc @alternatekev

Added hover styling on the dev/docs
@enejb enejb added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR labels Feb 1, 2016
@alternatekev
Copy link
Contributor

The reason why the twitter-alt is more for consistency.

To clarify: the naming convention for the other roundrect logos is this:

  • blogger.svg: roundrect
  • blogger-alt.svg: no roundrect
  • tumblr.svg: roundrect
  • tumblr-alt.svg: no roundrect

This PR just adds a roundrect twitter logo in place of the twitter.svg file (which is in accordance with the examples above), and moves what used to be twitter.svg over to twitter-alt.svg.

@alternatekev
Copy link
Contributor

Pinging @drw158 and @kellychoffman

@davewhitley
Copy link
Contributor

I'm fine with the naming convention, but maybe we should consider a more descriptive suffix like "-shape" instead. Some logos wouldn't have a suffixed version because the official logos already have a shape (eventbrite, FB, etc.). I'm fine with the alt suffix for now.

Share icon: In Gridicons we intentionally went with a different share icon.

If you don't have access to that link, basically the thought is that this share icon is not as recognizable, and apparently we are required to give attribution.
dcd29b00-0550-11e5-8a70-fd75268d348b

This is the one we went with for Gridicons:
55acfa66-0551-11e5-8000-8f6382ebf1ce

I think Gridicons and Social Logos should use the same share icon.

@kellychoffman
Copy link
Member

Do all the roundrect icons share the same border radius?

@alternatekev
Copy link
Contributor

I looked in the AI file and it seems like they’re all 2px border radius except Facebook, which is 1px. I think that’s their guideline.

@alternatekev
Copy link
Contributor

When this PR is merged, I'll be able to get #1474 past this stage (eg, with a roundrect Twitter logo):

screen shot 2016-02-01 at 2 59 48 pm

@davewhitley
Copy link
Contributor

Yep, Facebook has a smaller border radius; the vector is straight from their brand resources.

@alternatekev
Copy link
Contributor

@kellychoffman Is the border radius a blocker for this?

@kellychoffman
Copy link
Member

Not a blocker.

👍 Good to go.

@alternatekev alternatekev added Components and removed [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR labels Feb 2, 2016
@alternatekev
Copy link
Contributor

Code looks good to me, since it's generated in the social-logos repo.

@alternatekev alternatekev added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 2, 2016
alternatekev added a commit that referenced this pull request Feb 2, 2016
…tter-rect

Update: Social Logo to the latest version.
@alternatekev alternatekev merged commit 9e5f0fa into master Feb 2, 2016
@enejb enejb deleted the update/social-logos-with-twitter-rect branch February 2, 2016 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants