Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Add Venmo button #2376

Merged
merged 5 commits into from
May 12, 2014
Merged

Add Venmo button #2376

merged 5 commits into from
May 12, 2014

Conversation

thomasboyt
Copy link
Contributor

Four months after saying a Venmo pay button was "one to two weeks away" in #1922, we've finally gotten around to soft-launching our official button! I'd love for Gittip be one of its first users, since I think it's a much better flow than just linking to a user's profile.

This PR adds the Venmo button to the Venmo row in connected accounts:

screen shot 2014-05-09 at 4 53 02 pm

You can see the flow in action here (it's undergone a couple of visual tweaks since this):

http://quick.as/6m6s62j

This also fixes #2361.

Thomas Boyt added 4 commits May 6, 2014 22:52
Should be converted to use our SDK before hitting prod.
Conflicts:
	templates/connected-accounts.html
@thomasboyt thomasboyt changed the title Add venmo button Add Venmo button May 9, 2014
@@ -156,9 +164,6 @@ a.mini-user:hover {
}
}
}
tr.has-avatar > td {
vertical-align: bottom;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is needed, please add it back, you can use tr.has-avatar > .account-details as selector to prevent the vertical-align: middle on .account-action from being overridden.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm confused why you'd want this? Seems to totally break alignment to me.

Here it is with vertical-align: bottom

screen shot 2014-05-12 at 11 20 10 am

and without:

screen shot 2014-05-12 at 11 20 36 am

Copy link
Contributor

Choose a reason for hiding this comment

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

@thomasboyt The point is to align the platform's name with its logo, per #2265 (comment).

@Changaco
Copy link
Contributor

The rest of the PR looks okay to me. There are details I would change, but this PR's head is a remote branch so I can't add commits.

@thomasboyt
Copy link
Contributor Author

@Changaco FWIW, I mildly disagree on the vertical-align styling, but not enough to make a fuss about it. Reintroduced it so this PR can go forward :)

Changaco added a commit that referenced this pull request May 12, 2014
@Changaco Changaco merged commit f3806d3 into gratipay:master May 12, 2014
@thomasboyt thomasboyt deleted the add-venmo-button branch May 12, 2014 16:21
@chadwhitacre
Copy link
Contributor

@thomasboyt Thanks for getting back to this! :-) Did a blog post ever land?

@simon-weber
Copy link
Contributor

@chadwhitacre
Copy link
Contributor

Nice! :-)

Twitter

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Buttons in the "elsewhere" section of a profile are positioned incorrectly in Chrome when loading from cache
4 participants