fix(avatar): Fix misalignment on ie11 #676
Conversation
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
|
We will have to add some visual tests against this before merging. Someone on the core team can do that. But I just did a very similar fix so this PR looks like the right direction. We just got to be careful if borders can change width in different states. |
|
@lychyi I can try adding visual testing myself. |
| '&:not([disabled])': { | ||
| '&:focus': { | ||
| outline: 'none', | ||
| ...(variant === AvatarVariant.Dark ? focusRing({separation: 2}) : focusRing()), | ||
| ...focusRing({separation: 2}), |
There was a problem hiding this comment.
Note for posterity: I realized that there are no guarantees of the avatar color regardless of the variant so it doesn't make sense for this to be conditional
| [`[data-whatinput='mouse'] &:focus, | ||
| [data-whatinput='touch'] &:focus, | ||
| [data-whatinput='pointer'] &:focus`]: { | ||
| [`[data-whatinput="mouse"] &:focus, |
There was a problem hiding this comment.
Can we back out of the formatting changes here and in styled.ts below?
There was a problem hiding this comment.
These aren't formatting - this was intentional. I ran into issues with the StaticStates wrapper because it the regex was looking for " and not '.
I updated this to be more technically correct (attributes generally use "), and updated the regex in styled to catch either
| import {ComponentStatesTable, permutateProps} from '../../../../utils/storybook'; | ||
| import {Avatar} from '../index'; | ||
|
|
||
| const IMAGE_URL = 'https://s3-us-west-2.amazonaws.com/design-assets-internal/avatars/lmcneil.png'; |
There was a problem hiding this comment.
I think this is going to have to be sourced locally: https://www.chromatic.com/snapshot?appId=5d854c26ba934e0020f5e98a&id=5ecc4f8baabec60022686108
Resources requested by the new spec could not be loaded. Snapshots may be affected
Summary
fixes: #675
Checklist
yarn testpasses