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.

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

Tidy up profile sidebar #4374

Merged
merged 1 commit into from Aug 21, 2013

Conversation

Projects
None yet
9 participants
@svbergerem
Copy link
Member

svbergerem commented Aug 9, 2013

A first attempt to tidy up the profile sidebar.

mention_message_icons7

@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Aug 9, 2013

The smiley is about the mention?

@svbergerem

This comment has been minimized.

Copy link
Member Author

svbergerem commented Aug 9, 2013

@Flaburgan Yes, but I am not happy about that one. Just needed an icon and had no better one at the moment.

@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Aug 9, 2013

What about a simple @ ?

@svbergerem

This comment has been minimized.

Copy link
Member Author

svbergerem commented Aug 9, 2013

@Flaburgan that would have been way too easy ;-) Thanks for the advice, I updated the screenshot in the description.

@jhass

This comment has been minimized.

Copy link
Member

jhass commented Aug 9, 2013

Hm, tbh. this now looks line one button to me.

@Raven24

This comment has been minimized.

Copy link
Member

Raven24 commented Aug 9, 2013

I really like the concept, but I agree with @MrZyx. maybe it's enough if you space the icons a little more evenly and insert a white dash or something as a separator - and some kind of hover animation for clickable items would help UX.
Also ... would the green background become grey or something if the user is not sharing back? You could show a "X" instead of the check sign :)

@goobertron

This comment has been minimized.

Copy link

goobertron commented Aug 9, 2013

I love this.

I agree, it looks like one button. Perhaps the three icons could be spaced a little more, and perhaps each one in a slightly darker circle?

I assume that this button bar would be grey if this person is not sharing with me, and green if they are - is that right?

Anyway, really lovely concept.

I guess you might have to add 'block' in there as well, depending on how the discussion on #4304 goes...

@svbergerem

This comment has been minimized.

Copy link
Member Author

svbergerem commented Aug 9, 2013

@Raven24 nice suggestions :)

mention_message_icons2

@goobertron

This comment has been minimized.

Copy link

goobertron commented Aug 9, 2013

Yes! to this last series of images.

Only thing is I'm not sure about the x for 'not sharing with me'. An x suggests 'press this to cancel something' - is there anything we could use instead?

Other than that I think these look fantastic.

@DeadSuperHero

This comment has been minimized.

Copy link
Member

DeadSuperHero commented Aug 9, 2013

Wow, this looks great!

On Friday, August 9, 2013, goobertron wrote:

Yes! to this last series of images.

Only thing is I'm not sure about the x for 'not sharing with me'. An x
suggests 'press this to cancel something' - is there anything we could use
instead?

Other than that I think these look fantastic.


Reply to this email directly or view it on GitHubhttps://github.com//pull/4374#issuecomment-22392956
.

@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Aug 9, 2013

The x and the check should not be displayed at all: they are not action elements but information elements. The grey / green color is enough to have the information. And with #4304 there will be at least the "block" icon to display. But we should still find a way to precise to the user that green means the user is sharing with you.

@goobertron

This comment has been minimized.

Copy link

goobertron commented Aug 9, 2013

But we should still find a way to precise to the user that green means the user is sharing with you.

Fla, my guess is that that would be done (as with the icons) with a tool-tip.

@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Aug 9, 2013

@goobertron it's hard to put a tooltip on a container: it will constantly appear even hovering the other icons.

@svbergerem

This comment has been minimized.

Copy link
Member Author

svbergerem commented Aug 10, 2013

@Flaburgan what about something like that (already implemented but not pushed to github):
mention_message_icons3

@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Aug 10, 2013

That looks great. I think we should first merge #4304 and then you could integrate it in this PR.

@svbergerem

This comment has been minimized.

Copy link
Member Author

svbergerem commented Aug 10, 2013

Sounds good.

@goobertron

This comment has been minimized.

Copy link

goobertron commented Aug 10, 2013

This is looking really good. I just have some very small suggestions:

  • move the 'sharing'/'not sharing' icon into the centre of the bar, to give it symmetry. It looks slightly unbalanced with (in the left-hand example) the one other icon slightly to right of centre, or the other two icons not quite centred. It's a difficult one: I like what you're doing, and the sharing/not sharing icon doesn't really deserve equal space to the others, but it's just not quite right at the moment
  • I also think that the two icons in the right-hand example could be spaced a bit more evenly. They're close together, and with a lot of space on the outsides of them, it doesn't look quite right.
  • the cross for 'not sharing with me' doesn't work for me. It suggests either something is wrong, or that it needs to be clicked to cancel something. How about a circle as a symbol of nothing (like a zero)?
  • The bar as it is works really well with the square avatar, but if rounded corners are implemented for the avatars, you many need to rethink the design of the top edge. I guess then both top and bottom edges of the icon bar could be given the same radius rounding as the avatar.
@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Aug 10, 2013

About the first and the second point, I'd simply say center the icons (the first gray screen is okay, the second one I'd put the icons centered in their div.

Agree about the 3rd and the 4th point :)

@svbergerem

This comment has been minimized.

Copy link
Member Author

svbergerem commented Aug 10, 2013

I added some of your suggestions.

On the first and second image the buttons are centered on the remaining part of the div right of the sharing_message, on the third and fourth image the buttons are centered on the whole div. Which one do you prefer?

mention_message_icons4

@jhass

This comment has been minimized.

Copy link
Member

jhass commented Aug 10, 2013

I'd prefer the bottom two.

@goobertron

This comment has been minimized.

Copy link

goobertron commented Aug 10, 2013

Likewise, the bottom two look better to my eye as well, even though it means that the areas for the icons aren't all of equal width.

The only thing I'd suggest is to have the icons centred in their respective areas. As they appear in the right-hand image, when there are three icons, looks fine. When there are only two, they are a bit squashed towards the separator in the centre.

I'd guess that the best way to achieve this is to create a container div which doesn't include the area containing the circle/tick on the left, and also doesn't include the equivalent width on the right-hand side. Then the width of each of the divs containing the icons is the width of this container div divided by the number of icons; and the icons can be centred in their respective divs, however wide those divs have to be. (As usual when I'm trying to describe something technical I'm getting in a bit of a twist because I don't really have the technical vocabulary. Hope it makes sense, and is helpful.)

Anyway, this is great stuff - I love the design concept, and I think it's really going to enhance profile pages.

@svbergerem

This comment has been minimized.

Copy link
Member Author

svbergerem commented Aug 11, 2013

@goobertron again nice suggestions :) I updated the screenshot in the description.

@DeadSuperHero

This comment has been minimized.

Copy link
Member

DeadSuperHero commented Aug 11, 2013

For some reason, the buttons look a little bit off-center. I'd suggest trying to move the user-interaction buttons a little farther to the right, like so:

exmpl

@Raven24

This comment has been minimized.

Copy link
Member

Raven24 commented Aug 11, 2013

interesting, must be some sort of optical phenomenon :)
the icons centered in the dark-green box seems more balanced to me rather than centered on the whole green area.

@goobertron

This comment has been minimized.

Copy link

goobertron commented Aug 11, 2013

@svbergerem Those screenshots in your opening post now look perfect to me!

As there seems to be a split in opinion between centre icons on the whole green bar and centre icons on the dark green area, I can think of one simple solution, which is to reduce the opacity of the right-hand end of the area to match the left-hand end (the bit with the circle or tick). This will balance the bar visually and hopefully make icons centred on the whole bar look balanced to both 'factions' in this discussion. Does that work for everyone?

@svbergerem

This comment has been minimized.

Copy link
Member Author

svbergerem commented Aug 11, 2013

@goobertron I tried something different. First there was no right-hand area, then the area had a width of 20px. As there is a split in opinion I set the width of the area to 10px.

before (20px):
mention_message_icons6_before

after (10px):
mention_message_icons6_after

@Flaburgan

This comment has been minimized.

Copy link
Member

Flaburgan commented Aug 11, 2013

The second one is perfect!

@goobertron

This comment has been minimized.

Copy link

goobertron commented Aug 11, 2013

@svbergerem Brilliant - the 10px tricks my eyes perfectly!

@jhass jhass referenced this pull request Aug 11, 2013

Closed

Round user avatar #4288

@goobertron

This comment has been minimized.

Copy link

goobertron commented Aug 19, 2013

Do you think this will be ready to be merged for the next software release? I don't want to put on pressure, but it would be useful to know. If it is likely, I'll include it in the tutorials; if probably not, I'll omit it and update the tutorials when it's merged.

@svbergerem

This comment has been minimized.

Copy link
Member Author

svbergerem commented Aug 19, 2013

@goobertron: @sjain1107 wanted to work on #4304. When that work is finished I think I could make this PR ready to merge in a few hours.

@sjain1107

This comment has been minimized.

Copy link
Contributor

sjain1107 commented Aug 19, 2013

@svbergerem May I get just today?

@svbergerem

This comment has been minimized.

Copy link
Member Author

svbergerem commented Aug 19, 2013

goob would like to get this done until the next software release which will be AFAIK in 8 days so today sounds great :)

@goobertron

This comment has been minimized.

Copy link

goobertron commented Aug 19, 2013

I don't want to rush or pressure anyone! I just wanted an idea of how likely it was for these two PRs to be merged in time for the next software release, as I'm wondering whether to include details of the new design and ignore function in the tutorials I'm writing. If you think you can get them both done in the next few days, that's great!

@sjain1107

This comment has been minimized.

Copy link
Contributor

sjain1107 commented Aug 19, 2013

@goobertron Sure you can add the details of the ignore function in the tutorials :)

@goobertron

This comment has been minimized.

Copy link

goobertron commented Aug 19, 2013

Thanks, both of you.

@goobertron goobertron referenced this pull request Aug 21, 2013

Merged

Add tutorials pages #44

@jhass

This comment has been minimized.

Copy link
Member

jhass commented Aug 21, 2013

Tbh. I'm not confident #4304 will be ready for the next release in time. What about finishing this first and if they make it you can quickly integrate nonetheless?

@svbergerem

This comment has been minimized.

Copy link
Member Author

svbergerem commented Aug 21, 2013

What do you mean with finishing this? Should this get merged even if #4304 is not finished or should this be ready for a last-minute merge if #4304 is ready in time? If it is the first one should I implement the functionality of the block button by myself or should I completely remove the block button from this PR?

@goobertron

This comment has been minimized.

Copy link

goobertron commented Aug 21, 2013

If it's not difficult to implement the block button and functionality in this PR, I'd be happy for that. As long as it won't be treading on anyone else's toes.

It would be a shame to launch a 'part-finished' version of the new profile page because it's waiting for one element to be finished elsewhere. Would it be possible/desirable for Steffen to help Sakshi get her PR finished quickly?

@jhass

This comment has been minimized.

Copy link
Member

jhass commented Aug 21, 2013

My first thought was merging this without the button and integrating the button when it's there. But if you think it's better to not have this without the button I'll remove the milestone here and we'll see what happens.

@svbergerem

This comment has been minimized.

Copy link
Member Author

svbergerem commented Aug 21, 2013

Just looked again at the code (and rebased ;) ). I could remove the ignore button but then we will have empty green/grey/red bar under the profile image with a small check/circle on the left side if you are not following or ignoring that person. IMO that won't look bad but I'd like to hear some opinions on that.

@goobertron

This comment has been minimized.

Copy link

goobertron commented Aug 21, 2013

I don't think we should hold up a good, functioning feature because of one small element that isn't ready. It would be a shame not to release it all at once, but I think this feature is 'complete' even without the block function and button. It may look very slightly odd without the button in place, but I guess people who don't know that the button should be there won't notice!

I guess we can do what we can to help Sakshi finish her PR in time, but if that's not possible, I think we should merge this one anyway, as long as there are no conflicts without the blocking code in place.

@svbergerem

This comment has been minimized.

Copy link
Member Author

svbergerem commented Aug 21, 2013

I removed the ignore button by commenting out some lines so implementing it back again should be pretty easy.
This is how it looks like:
mention_message_icons8

@goobertron

This comment has been minimized.

Copy link

goobertron commented Aug 21, 2013

Brilliant. Because the icons are centred in the divs, it actually doesn't notice that one of them is 'missing'. And then when blocking on the profile page is implemented, that can be introduced as a new feature, along with the addition of the icon in this bar. Great work, Steffen.

@svbergerem

This comment has been minimized.

Copy link
Member Author

svbergerem commented Aug 21, 2013

Thanks. I still hope that #4304 is ready in time. Would be great to have that feature in the next release.

@DeadSuperHero

This comment has been minimized.

Copy link
Member

DeadSuperHero commented Aug 21, 2013

Looks fantastic! Wonderful work @svbergerem :)

Steffen van Bergerem

jhass added a commit that referenced this pull request Aug 21, 2013

@jhass jhass merged commit a365d29 into diaspora:develop Aug 21, 2013

1 check passed

default The Travis CI build passed
Details
@jhass

This comment has been minimized.

Copy link
Member

jhass commented Aug 21, 2013

Thank you!

@svbergerem svbergerem deleted the svbergerem:profile-message-and-mention-icons branch Aug 21, 2013

@sjain1107

This comment has been minimized.

Copy link
Contributor

sjain1107 commented Aug 22, 2013

@svbergerem Great work !! I'll try my best to implement the ignore user icon. Need your help.

@svbergerem

This comment has been minimized.

Copy link
Member Author

svbergerem commented Aug 22, 2013

@sjain1107 Sure, just ask what you need to know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.