fix(avatar): font icons sizing issue #846
Conversation
kuzhelov
requested review from
dvdzkwsk,
layershifter,
levithomason,
miroslavstastny and
mnajdova
as code owners
| @@ -1,6 +1,10 @@ | |||
| import { Func } from '../types' | |||
|
|
|||
| // https://jsperf.com/startdust-callable | |||
There was a problem hiding this comment.
we may safely remove this reference as
- it is pointing to stale document
- we have all the necessary tooling now to measure performance on the Stardust's side
|
|
||
| import { sizeToPxValue as avatarSizeToPxValue } from '../Avatar/avatarStyles' | ||
|
|
||
| const sizeToIconPaddingInPx: Record<SizeValue, number> = { |
There was a problem hiding this comment.
these values need to be refined
Generated by 🚫 dangerJS |
Codecov Report
@@ Coverage Diff @@
## master #846 +/- ##
=======================================
Coverage 93.54% 93.54%
=======================================
Files 21 21
Lines 728 728
Branches 69 73 +4
=======================================
Hits 681 681
Misses 47 47Continue to review full report at Codecov.
|
| @@ -1,8 +1,8 @@ | |||
| import { pxToRem } from '../../../../lib' | |||
| import { ComponentSlotStylesInput, ICSSInJSStyle } from '../../../types' | |||
| import { AvatarProps } from '../../../../components/Avatar/Avatar' | |||
| import { AvatarProps } from '../../../..' | |||
There was a problem hiding this comment.
Can we still use the direct import?
There was a problem hiding this comment.
sure, thanks for catching this thing!
|
|
||
| [`&.${Avatar.slotClassNames.image}`]: { | ||
| ...(isFontBased && getAvatarFontIconStyles(size, v)), | ||
| }, |
There was a problem hiding this comment.
Actually, this is a single thing that stops me from the approve button.
I understand why Avatar can know about Icon, because it's a slot of Avatar. But, I want to avoid the reserve coupling because Icon is a primitive component 🤔
This PR fixes appearance of
Iconcomponent when it is provided intoimageslot ofAvatarcomponent (originally reported here #795):Was
Now