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

Use new Facebook color and glyph #745

Merged
merged 9 commits into from
Jul 19, 2019
Merged

Use new Facebook color and glyph #745

merged 9 commits into from
Jul 19, 2019

Conversation

dnywh
Copy link
Contributor

@dnywh dnywh commented Jul 9, 2019

📲 What

  • Updated the fillColor for our Facebook button (Log in with Facebook)
  • Updated the asset for the Facebook glyph for the Log in with Facebook button

🤔 Why

Facebook updated its brand guidelines in April this year.

🛠 How

  • Asset: Swapped out the PDF file
  • Color: amended the Facebook color variable

👀 See

Trello, screenshots, external resources?

Before 🐛 After 🦋
Simulator Screen Shot - iPhone SE - 2019-07-09 at 17 27 16 Simulator Screen Shot - iPhone SE - 2019-07-18 at 13 57 02

⏰ TODO

  • Pair with someone on changing the asset type from 1x, 2x, 3x, to 'All'
  • Fix color list in JSON
  • Pair with someone on fixing the stretched-out quirk on the glyph

@dnywh dnywh requested a review from cdolm92 July 10, 2019 17:26
@dnywh dnywh changed the title Use new Facebook color and glyph [WIP] Use new Facebook color and glyph Jul 10, 2019
@dnywh dnywh changed the title [WIP] Use new Facebook color and glyph Use new Facebook color and glyph Jul 18, 2019
@dnywh dnywh assigned dnywh and cdolm92 and unassigned dnywh Jul 18, 2019
dusi
dusi previously approved these changes Jul 18, 2019
Copy link
Contributor

@dusi dusi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me...but would still wait for Christella to sign off if she finds everything OK before merging.

Another PR merged Danny! 🎉

@dusi dusi dismissed their stale review July 18, 2019 20:47

Looks like tests are still failing

? .init(topBottom: 10.0, leftRight: 12.0)
: .init(topBottom: 12.0, leftRight: 16.0)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dusi and I realised that we needed to have a custom topBottom for this case. That was causing the visually-squashing problem and why we declare it here.

Copy link
Contributor

@dusi dusi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢 🚢 🚢
🚢 🚢 🚢
🚢 🚢 🚢

@dnywh dnywh merged commit 8d98b90 into master Jul 19, 2019
@dnywh dnywh deleted the facebook-button-update branch July 19, 2019 16:13
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.

3 participants