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

🔵 Use theming color as group and shared call colors instead of bland grey #1274

Closed
wants to merge 1 commit into from

Conversation

jancborchardt
Copy link
Member

We might have discussed this, I think @Ivansss it was us? :) This makes it look much nicer instead of the default grey color:

grey placeholder icon
placeholder color to primary

Also much nicer in the dropdowns:
blue avatars in dropdown

And for the email sharing, the icon is changed to white to fit better:
email avatars blue

Please review @nextcloud/talk 🎉

Could you also adjust that in the mobile Talk apps cc @Ivansss @mario?

Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

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

The icons should be dark when the theming colour is light, like done in the header icons.

I would add too a tiny border to the "avatars" with the same colour as the icon, so it is usually "invisible" (due to being white on white), but frames the avatar shape when the colour of the icon is light.

talk-theming-color-white

@jancborchardt
Copy link
Member Author

The icons should be dark when the theming colour is light, like done in the header icons.

Can you help with that? :) Or how do I do that?

I would add too a tiny border to the "avatars" with the same colour as the icon, so it is usually "invisible" (due to being white on white), but frames the avatar shape when the colour of the icon is light.

This wouldn’t look so good because it does become visible on hover/focus of the entry cause the background switches from white to grey. Or what do you mean? It’s fine if when you use white for theming, there’s simply seemingly no circle around the icon. No border needed.

@danxuliu
Copy link
Member

danxuliu commented Nov 2, 2018

The icons should be dark when the theming colour is light, like done in the header icons.

Can you help with that? :)

Of course! Let me help you by summoning @juliushaertl and @skjnldsv :-P

This wouldn’t look so good because it does become visible on hover/focus of the entry cause the background switches from white to grey.

Mmmm, right, I did not think about that ;-)

It’s fine if when you use white for theming, there’s simply seemingly no circle around the icon. No border needed.

👍

@skjnldsv
Copy link
Member

skjnldsv commented Nov 3, 2018

You need to use the icon color api and the $color-primary-text as the color value :)
So basically redeclare every icon you might use on spreed:

.icon-link {
	@include icon-color('link', 'places', $color-primary-text, 2, true);
}

and so on :)

@nickvergessen
Copy link
Member

In my opinion this makes it too colorful.
Drawing attention is what the primary color is supposed to do. But for those icons we don't need to do this.

@danxuliu
Copy link
Member

I agree with @nickvergessen; if the primary colour is overused it loses its value to draw attention over the most important elements.

I would be totally fine with the grey icons, but if you want to add more colour, maybe it would be better to use a light variant of the primary colour, or a slight variation on hue? Or maybe introduce a secondary colour, like the complimentary or one of the triads of the main colour?

@nickvergessen
Copy link
Member

Closing as per above

@nickvergessen nickvergessen deleted the placeholder-color branch November 14, 2018 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants