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

[Avatar] remove border on images and use boxShadow #1204

Merged
merged 1 commit into from
Jul 20, 2015
Merged

[Avatar] remove border on images and use boxShadow #1204

merged 1 commit into from
Jul 20, 2015

Conversation

oliviertassinari
Copy link
Member

I can't find the border on the spec of material design.

@hai-cea
Copy link
Member

hai-cea commented Jul 19, 2015

@oliviertassinari When I was implementing lists, I notice the image Avatars have borders. Please see:
image

@oliviertassinari
Copy link
Member Author

Ok but I feel like the border is less than 1px on this example.

On the project I'm working on, I have two issues.
The border is not present on Letter Avatar. And I feel like the border is too thick.
A box shadow inset may be more appropriate.
Somethings like box-shadow: 0 0 5px 1px rgba(0, 0, 0, 0.7) inset;

@oliviertassinari
Copy link
Member Author

@hai-cea What do you think?

@hai-cea
Copy link
Member

hai-cea commented Jul 19, 2015

@oliviertassinari I think the box-shadow sounds good.

@oliviertassinari oliviertassinari changed the title [Avatar] remove border on images [Avatar] remove border on images and use boxShadow Jul 19, 2015
@oliviertassinari
Copy link
Member Author

@hai-cea Updated. Left : this PR, right: master
image

I can't find the border on the spec of material design.
@hai-cea
Copy link
Member

hai-cea commented Jul 20, 2015

Thanks @oliviertassinari - looks great. 👍

hai-cea pushed a commit that referenced this pull request Jul 20, 2015
[Avatar] remove border on images and use boxShadow
@hai-cea hai-cea merged commit 71adea6 into mui:master Jul 20, 2015
@JAStanton
Copy link
Contributor

"remove border on images and use boxShadow" <-- border fast, boxShadow slow. you guys are trading asthetics for speed right now. Just saying. Maybe it's not a big deal but in the future. BoxShadows are terribly slow to render

@oliviertassinari
Copy link
Member Author

I agree it's slower.
@JAStanton Have you noticed performance issues with scrolling?
In my use case, I set the boxShadow to none.

@hai-cea
Copy link
Member

hai-cea commented Jul 22, 2015

I think just changing the border color instead of using box-shadow could do the trick. Here are my tests - what do you think?

//new border color
border: solid 1px rgba(0,0,0,0.08)
image

//this PR
box-shadow: 0 0 1px 0 rgba(0, 0, 0, 0.2) inset
image

//current on material-ui.com
border: solid 1px #e0e0e0
image

@JAStanton
Copy link
Contributor

Looks good to me, and if anyone wants more they can always override themselves and take the perf penalty

@oliviertassinari
Copy link
Member Author

@hai-cea Ok, why not. let's kill it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: avatar This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants