#4288 - implementation of a round user avatar

[juliaguar]

just a quick design change to implement a round user avatar (as mentioned in issue #4288).

[Flaburgan]

Can you please add the box shadow too?
box-shadow: 0px 1px 2px #666;

[jhass]

We can talk about the shadow but I still don't like the rounded corners. Please note that's just my opinion, if the majority thinks it looks better, it'll go in.

[goobertron]

I quite like rounded corners. It seems to work well for the avatars in Github! ;-)

[Flaburgan]

@juliaguar the important point of this issue is to unify the look of the avatar everywhere. Could you please complete your PR by factorizing this change with the other place where an avatar is displayed?

A search of .avatar in application.css.sass points different places where there is different style, look at https://github.com/diaspora/diaspora/blob/develop/app/assets/stylesheets/application.css.sass#L1177 and https://github.com/diaspora/diaspora/blob/develop/app/assets/stylesheets/application.css.sass#L1197 for example.

[juliaguar]

@Flaburgan I checked the application and there is no need to change it in other places, because it inherits from the .avatar where I changed it...

[Flaburgan]

Can you please also remove https://github.com/Team-D/diaspora/blob/55b39aaafbe25802a9475d67deeab9be88523558/app/assets/stylesheets/application.css.sass#L1181

Then, add a changelog entry in Changelog.md
Then, git rebase -i develop (squash all your commit except the first one, let "pick")
And I think it'll be okay :)

[juliaguar]

@Flaburgan Thx for the advice. I updated the Changelog.md, but I would rather not touch the history unless it is really necessary, because everywhere they say, that it is considered bad practice to change it.

[Flaburgan]

It's not about the whole history but only about your commits: we want to merge only one commit from you to keep a clean git log history. We ask that for every PR ;)

[DeadSuperHero]

Okay, looks like this has all been rolled into one commit.

Good to merge?

[Flaburgan]

We're still discussing about the rounded corner on #4288
Especially fixed values don't feel great with small image, we need to put a % value.

[fabianrbz]

@juliaguar we finally decided to go with only shadow, full discussion here

[goobertron]

I think Julia has finished working on Diaspora now, as her work was part of the 'Rails Girls Summer of Code', which has now ended.

Does anyone want to pick up this PR?

[juliaguar]

Hey, sry for not responding... I will update the PR next week.

[fabianrbz]

@juliaguar no worries, thanks!

[juliaguar]

Is it possible to merge the PR like this?
I tried to do a git rebase -i develop, but run into a lot of merge conflicts, because the branch wasn't updated for 5 month... But if it can't be merged like this I could try to reapply the changes on a new branch.

[jhass]

Reapplying the changes is probably the easiest thing to do for this amount of them. Here's a little trick to circumvent opening a new PR:

git branch -m feature/4288-round_user_avatar feature/4288-round_user_avatar_old
git checkout develop
git pull upstream develop
git checkout -b feature/4288-round_user_avatar
# Reapply and commit changes
git push -uf origin feature/4288-round_user_avatar

[jhass]

Adding some screenshots

Anyone objects?

[fabianrbz]

👍

[fabianrbz]

@juliaguar could you please rebase?

[goobertron]

Is this one up for adoption? I'll have a go at it if so.

[jhass]

Maybe we should look into this again once we're through #4657

[goobertron]

Yes, I wondered about that. But I went ahead and did it anyway, as an exercise to teach me a little bit more about how the code's put together. I can push it if you'd like, or wait to see what 4657 does.

[jhass]

Do as you like ;)

[goobertron]

OK, I did! :p

[jaywink]

Do I understand right #4887 supersedes this one? If so I'll close this

[goobertron]

It does, but it hasn't been accepted yet and needs some more work. Probably best to leave this open until the issue has been fixed and merged, just in case my solution doesn't get accepted.

[svbergerem]

Superseded by #5733. Thank you for your work.