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 Facebook icon SVG #9178

Merged
merged 1 commit into from Aug 21, 2018

Conversation

Projects
None yet
4 participants
@kjellr
Contributor

kjellr commented Aug 20, 2018

Description

#8916 added some custom block icons, including a facebook icon. For some reason, the facebook icon is missing most of its curved lines, and its proportions are incorrect. This PR replaces it with a new SVG.

Current icon on the left, new icon on the right:

screen shot 2018-08-20 at 3 46 35 pm


Before, in context:

screen shot 2018-08-20 at 3 44 37 pm

screen shot 2018-08-20 at 3 44 57 pm

After, in context:

screen shot 2018-08-20 at 3 43 43 pm

screen shot 2018-08-20 at 3 47 01 pm


This icon was sourced from https://github.com/Automattic/social-logos, and I'm fairly certain it follows the same icon grid we've been using. cc @jasmussen for a gut check there though.

@kjellr kjellr self-assigned this Aug 20, 2018

@kjellr kjellr requested a review from jasmussen Aug 20, 2018

@jasmussen

I really kind of hate that we are becoming the maintainers of 3rd party icons. IMO that is their domain, and we shouldn't have to maintain it.

However given that we have an Embed block for it, we might as well not let the user experience suffer. So ship it, and nice work!

@karmatosed

This comment has been minimized.

Show comment
Hide comment
@karmatosed

karmatosed Aug 21, 2018

Member

+1 ship it but yes it's hard for us to maintain this.

Member

karmatosed commented Aug 21, 2018

+1 ship it but yes it's hard for us to maintain this.

@kjellr kjellr merged commit 3ee6407 into master Aug 21, 2018

2 checks passed

codecov/project 50.92% remains the same compared to bc4d7a2
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@kjellr kjellr deleted the update/facebook-icon branch Aug 21, 2018

@kjellr

This comment has been minimized.

Show comment
Hide comment
@kjellr

kjellr Aug 21, 2018

Contributor

Thanks, @karmatosed + @jasmussen!

Contributor

kjellr commented Aug 21, 2018

Thanks, @karmatosed + @jasmussen!

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Aug 22, 2018

Member

@kjellr Can we make sure merged pull requests are assigned to milestones, please?

Member

danielbachhuber commented Aug 22, 2018

@kjellr Can we make sure merged pull requests are assigned to milestones, please?

@kjellr

This comment has been minimized.

Show comment
Hide comment
@kjellr

kjellr Aug 22, 2018

Contributor

Sure thing, @danielbachhuber. Sorry about that — first merge in this repo. 🙂 Should I assign a milestone to this PR after the fact? (I'm guessing 3.7 is fine for this one).

Contributor

kjellr commented Aug 22, 2018

Sure thing, @danielbachhuber. Sorry about that — first merge in this repo. 🙂 Should I assign a milestone to this PR after the fact? (I'm guessing 3.7 is fine for this one).

@danielbachhuber

This comment has been minimized.

Show comment
Hide comment
@danielbachhuber

danielbachhuber Aug 22, 2018

Member

@kjellr Congrats on the first merge :) Yes, please assign whichever milestone is appropriate.

There are some more details generally in this document: https://github.com/WordPress/gutenberg/blob/master/docs/reference/repository-management.md#merging-pull-requests If you find the document could be improved in some way, feel free to open an issue with the questions you have, or pull request with suggested changes.

Member

danielbachhuber commented Aug 22, 2018

@kjellr Congrats on the first merge :) Yes, please assign whichever milestone is appropriate.

There are some more details generally in this document: https://github.com/WordPress/gutenberg/blob/master/docs/reference/repository-management.md#merging-pull-requests If you find the document could be improved in some way, feel free to open an issue with the questions you have, or pull request with suggested changes.

@kjellr kjellr added this to the 3.7 milestone Aug 22, 2018

@kjellr

This comment has been minimized.

Show comment
Hide comment
@kjellr

kjellr Aug 22, 2018

Contributor

Awesome, thank you @danielbachhuber!

Contributor

kjellr commented Aug 22, 2018

Awesome, thank you @danielbachhuber!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment