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

Improve display of connected accounts and avatars. #2265

Merged
merged 9 commits into from
Apr 29, 2014
Merged

Conversation

duckinator
Copy link
Contributor

Improve the display of Connected Accounts and avatars.

Some notes:

  • If an account is not linked it'll show the generic avatar instead of nothing (Only show actually connected accounts #2196).
  • Things without images: these seem to be special cased anyway (Bitcoin, Balanced), and thus don't need to be handled, as far as I know.
  • Although I advocated for having the avatar be the smaller one, after trying that, the avatars proved to look horrible, so I made them the larger of the two pictures.

Closes #2154.

gittip-issue-2154-fix-screenshot

@chadwhitacre
Copy link
Contributor

Nice! Can we tighten up the layout a bit?

  1. Part of what was going on in the original mock is that the text name of the platform was right next to the logo for the platform.
  2. Another thing was that the whitespace above and below the avatar was equal. The avatar was centered between the borders.
  3. The platform logo is positioned such that the white circle around the logo touches the bottom border.
  4. The baseline of the platform username is centered between the borders (baseline extended would run through the "x" at right)

connected-accounts-sketch

@chadwhitacre
Copy link
Contributor

Also, I don't think we should show the generic avatar for unconnected accounts. After all, it's a Gittip avatar, so it's odd to have it represent an unknown Venmo (e.g.) account. I think we should show the platform logo in the large square and drop the round img entirely for this case.

@duckinator
Copy link
Contributor Author

@whit537 but shouldn't unconnected accounts not be shown at all, thus making it a nonissue?

@chadwhitacre
Copy link
Contributor

@duckinator Currently we show them on one's own profile, with an "Add" button.

@duckinator
Copy link
Contributor Author

@whit537 there was a bit of an issue with it covering up too much of the image. For your avatar, it'd probably cover like half of your face. :P

I tweaked it a bit (more whitespace above and below it, and moved the icon a bit further off of the avatar), but can't seem to make it look nice if I do that. Thoughts?

gittip-issue-2154-fix-screenshot

@clone1018
Copy link
Contributor

I like it, and it works well for me, @duckinator is this ready?

@duckinator
Copy link
Contributor Author

@clone1018 I'm still doing some CSS cleanup and such locally. Got a bit distracted by other things yesterday. :)

I'll finish it up and @mention you with a final screenshot when it's ready!

@clone1018
Copy link
Contributor

Do you think you can fit in a fix for #2271 ? I started on one and then realized this PR would conflict :(

@clone1018
Copy link
Contributor

This easiest fix would be removing position: absolute;

@duckinator
Copy link
Contributor Author

I'll take a look after finishing cleaning up the slight CSS mess I have left. :)

@duckinator
Copy link
Contributor Author

Looks like my code made that even more broken, and fixing that should also fix #2271 lol

@chadwhitacre
Copy link
Contributor

@duckinator I'd still like to see the changes I mentioned at #2265 (comment). If the platform logo is too big we can shrink it a little bit, but the four points I made there still apply afaict.

@duckinator
Copy link
Contributor Author

Sorry for kind of falling off the radar with this. Planning to rebase it off master and pick it back up later today.

@clone1018
Copy link
Contributor

I can't wait!

@duckinator
Copy link
Contributor Author

Rebased.

I have no clue why it decided to list all of the crap from master here.

Can I throw git out of a window, now?

@seanlinsley
Copy link
Contributor

@duckinator what commands did you run?

@duckinator
Copy link
Contributor Author

It would appear that I somehow managed to rebase before fetching master. DON'T EVEN KNOW.

@chadwhitacre
Copy link
Contributor

IRC re: git shenanigans, ftr.

@seanlinsley
Copy link
Contributor

BTW @duckinator this needs to be rebased on master. There's a merge conflict, likely because of #2307

@Changaco
Copy link
Contributor

I have rebased this on master and made it look more like the original mockup.

clone1018 added a commit that referenced this pull request Apr 29, 2014
Improve display of connected accounts and avatars.
@clone1018 clone1018 merged commit 58265c4 into master Apr 29, 2014
@clone1018 clone1018 deleted the issue-2154 branch April 29, 2014 22:20
@duckinator
Copy link
Contributor Author

Thanks @Changaco!

@Changaco Changaco mentioned this pull request May 12, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve display of Connected Account + Avatar
5 participants