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

[NT-76] Facebook Log In Copy Updates #880

Merged
merged 22 commits into from
Oct 10, 2019
Merged

[NT-76] Facebook Log In Copy Updates #880

merged 22 commits into from
Oct 10, 2019

Conversation

cdolm92
Copy link
Contributor

@cdolm92 cdolm92 commented Oct 8, 2019

📲 What

Facebook log in copy updates in log in/sign up screen for pledge flow.

🛠 How

Facebook button title can either be Continue with Facebook when logged out user is pledging or Log in with Facebook when logging in from anywhere else. This is determine w/ the LoginIntent

👀 See

Before 🐛 After 🦋
Simulator Screen Shot - iPhone Xs - 2019-10-08 at 18 23 43 Simulator Screen Shot - iPhone Xs - 2019-10-08 at 15 05 41

✅ Acceptance criteria

  • Navigate to a project and tap on the Back this Project CTA, tap Continue sign up screen should show and facebook button title should be Continue with Facebook
  • Facebook button title outside of pledge flow should say Log in with Facebook

⏰ TODO

  • Feedback from PM
  • Fill the PR w/ ACs

_ = self.getNotifiedLabel
|> baseLabelStyle
|> UILabel.lens.font %~~ { _, label in
label.traitCollection.isRegularRegular ? .ksr_headline(size: 15.0) : .ksr_headline(size: 12.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.

Opted for headline font here because caption1 caused it wrap due to trucation.

Simulator Screen Shot - iPhone Xs - 2019-10-08 at 15 03 18

@cdolm92 cdolm92 changed the title Fb cta copy changes [NT-76] Facebook Log In Copy Updates Oct 8, 2019
@cdolm92 cdolm92 marked this pull request as ready for review October 8, 2019 22:25
@cdolm92 cdolm92 requested a review from ifbarrera October 8, 2019 22:29
Copy link
Contributor

@ifbarrera ifbarrera left a comment

Choose a reason for hiding this comment

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

I noticed that the PR description says that the facebook button's title should be "Login with Facebook" with the .login intent and Continue with Facebook with the .backProject intent, but it looks like in the screenshots the Facebook button says "Login" 🤔

testLoginToutView_intent_generic_lang_en_device_phone4_7inch@2x

Can you clarify which one it should be?

_ = self.getNotifiedLabel
|> baseLabelStyle
|> UILabel.lens.font %~~ { _, label in
label.traitCollection.isRegularRegular ? .ksr_headline(size: 15.0) : .ksr_headline(size: 12.0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we try to use a font that doesn't need a size override? So maybe .ksr_subhead().bolded and .ksr_caption1().bolded?

Copy link
Contributor

@ifbarrera ifbarrera left a comment

Choose a reason for hiding this comment

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

lgtm!

@cdolm92 cdolm92 merged commit e179cae into master Oct 10, 2019
@cdolm92 cdolm92 deleted the fb-cta-copy-changes branch October 10, 2019 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants