Code review comment for lp:~bac/launchpad/bug-436980-mugshots

Revision history for this message
Martin Albisetti (beuno) wrote :

Hi Brad, Michael, it's great to see this page get some love.
Michael's comment on the needed padding is spot on, that would make the UI good to land for me as well.

I do however have 3 comments about this page, that if addressed, probably belong in a follow-up branch:

1) I see this page used frequently to show off the team. By showing it in batches of 8, it will not really be possible for a team larger than 8. Could we address that in any way? I understand that "see all" will timeout, but it's a shame we loose that "feature". Maybe middle ground would be larger batches. I don't think scrolling is an issue here, so 24 or even 32 wouldn't be too bad.

2) This page is only really useful if people have mugshots, otherwise it's pretty useless. Could we order alphabetically, but showing the people with pictures first?

3) Last but not least, if a user hasn't set their mugshot, it would be super cool if they could when they see they don't have one on this page. Maybe just a "Upload a picture" link under his empty mugshot?

review: Approve (ui)

« Back to merge proposal