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.

Sign up for GitHub

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

Jump to bottom

Recommendations info dialog #255

Merged
merged 5 commits into from May 23, 2018
Merged

Recommendations info dialog #255

merged 5 commits into from May 23, 2018

Conversation

eoji
Copy link
Contributor

@eoji eoji commented May 23, 2018

what

Showing recommendations info, with tests (•̀ᴗ•́)و ̑̑

how

Updated strings.
Added the new info icon.
On user click, we show the alert.
Test ✨

why

GIF

c-c-c-check it out

2018-05-23 14_56_24

cc @dannyalright

@eoji eoji added the bug 🐛 label May 23, 2018
@eoji eoji added GDPR 🔐 and removed bug 🐛 labels May 23, 2018
@dnywh
Copy link
Contributor

dnywh commented May 23, 2018

minor thing: I'd expect the Got it. to be all caps on Android.

  1. would you, too?
  2. is this consistent with how we label dialog buttons elsewhere on the app?

@eoji
Copy link
Contributor Author

eoji commented May 23, 2018

@dannyalright

  1. I actually would expect it to be in caps, per https://material.io/design/components/dialogs.html#alert-dialog
  2. Yes, our other dialog buttons have sentence case 🤔

@dnywh
Copy link
Contributor

dnywh commented May 23, 2018

cool. I'd prefer it to be GOT IT, without the period at the end. Should we do it here, one-off, and I'll make a card to fix others another time? Or just keep as is and do all at another time?

@eoji
Copy link
Contributor Author

eoji commented May 23, 2018

@dannyalright Let's do it now and I'll open a PR with that new string!

@eoji
Copy link
Contributor Author

eoji commented May 23, 2018

Here's the strings PR: https://github.com/kickstarter/kickstarter/pull/11028

@eoji
Copy link
Contributor Author

eoji commented May 23, 2018

New string 😎 @dannyalright
2018-05-23 17_46_23

@dnywh
Copy link
Contributor

dnywh commented May 23, 2018

awesome, thank you!

Scollaco
Scollaco approved these changes May 23, 2018
View reviewed changes
Copy link
Contributor

@Scollaco Scollaco left a comment

Choose a reason for hiding this comment

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

Nice dialog. I recommend it! 👍

@eoji eoji merged commit 13e5c5b into master May 23, 2018
@eoji eoji deleted the recs-info branch May 23, 2018 22:39
@eoji
Copy link
Contributor Author

eoji commented May 23, 2018

Recommendations and Privacy UI in Settings

Rcureton pushed a commit that referenced this pull request Jul 31, 2018
@eoji @luoser
login tout string update (#255)
15e6ce6
Rcureton pushed a commit that referenced this pull request Jul 31, 2018
@eoji
Recommendations info dialog (#255)
b548983 
* showing recommendations info with tests (•̀ᴗ•́)و ̑̑
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Scollaco Scollaco Scollaco approved these changes

@ifbarrera ifbarrera Awaiting requested review from ifbarrera

@cdolm92 cdolm92 Awaiting requested review from cdolm92

Assignees
No one assigned
Labels
GDPR 🔐
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@eoji @dnywh @Scollaco