[Avatar] Use semi-transparent border

[mbrookes]

• PR has tests / docs demo, and is linted.
• Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
• Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

Update image Avatar to have a semi-transparent border per MD spec.

[oliviertassinari]

That looks good to me 👍

[pdf]

Where does the spec specify color? What happens if someone wants to display an avatar on an rgb(128,128,128) background? The ColorManipulator.fade() call was already setting transparency.

[mbrookes]

Where does the spec specify color?

It doesn't - you'll have to closely examine the examples.

What happens if someone wants to display an avatar on an rgb(128,128,128) background?

What do you mean by 'what happens'? Have you tried? What happened? What would you expect to happen instead?

[pdf]

What do you mean by 'what happens'? Have you tried? What happened? What would you expect to happen instead?

Well, I haven't tried, but the border would become invisible, no? I went through and cleaned out all the hard-coded colour values in this code-base some time ago so that themes would work properly. Pretty sure I chose textColor here because it will always be visible. If textColor is not the right value, perhaps some other variable would suit? If it's just the opacity that needs adjusting, then it should be adjusted in the fade call. The commit doesn't entirely describe to me what the fix was aiming at.

[mbrookes]

Well, I haven't tried

Best not to make assumptions then. 😄

but the border would become invisible, no?

No.

I went through and cleaned out all the hard-coded colour values

It isn't hard coded - it can be overridden both in the theme, and also the style or className prop (if someone wants a solid or colored border for example).

Calculating the colour of a semi-transparent border over the image from textColor is a bit obtuse. We could add an alpha grey to colors if it would make you feel better? 😁

[pdf]

On 04/10/2016 22:30, Matt Brookes wrote:

Well, I haven't tried

Best not to make assumptions then. 😄

This is a very reasonable assumption.

but the border would become invisible, no?

No.

Umm, yes? How can a transparent version of the same color as the background be visible?

Here: http://jsbin.com/fizidahaci/edit?html,css,output

Can you see the border on the second div?

I went through and cleaned out all the hard-coded colour values

It isn't hard coded - it can be overridden both in the theme, and also
the |style| or |className| prop if someone really wants a colored
border or similar. Calculating the colour of a semitransparent border
over the image from |textColor| is a bit obtuse.

Overrides of the latter sort only work per-instance, not application-wide, whereas if it's based on the palette, it's very easy to update consistently, and if based on textColor is guaranteed to be visible, unless the user makes their text invisible.

[oliviertassinari]

Overrides of the latter sort only work per-instance, not application-wide

As the color is in the theme, it can be changed once for the whole application.

The question is whether or not should we use the textColor to compute the borderColor. I would say no.
E.g. I think that with a textColor white, it's better to have a dark borderColor.

[pdf]

As the color is in the theme, it can be changed once for the whole application.

I was referring to using style/className, the new getMuiTheme functionality does appear to make this significantly less of an issue though.

E.g. I think that with a textColor white, it's better to have a dark borderColor.

If you have textcolor white, it's because you have a dark background color though, making a dark border less visible. 50% grey is probably the most widely useful color at least though.

Is the idea to replace all the theme elements that are based on modified textColor/canvasColor? I need to plan for theme updates if there are more changes of this sort incoming.

[oliviertassinari]

Is the idea to replace all the theme elements that are based on modified textColor/canvasColor? I need to plan for theme updates if there are more changes of this sort incoming.

There is no plan, that I'm aware of, into that direction.
I think that we should use textColor and canvasColor only where it makes sense.
I agree with you, using those two colors is often a good call.