-
Notifications
You must be signed in to change notification settings - Fork 21
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
fix border-radius #255
fix border-radius #255
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, it def looks more consistent that way.
There's just a little thing that I made a suggestion for. If that work I think it's better, if not we can forget as this will be changed eventually.
@@ -141,7 +141,7 @@ export default styled(Transaction)( | |||
max-height: 1.3125rem; | |||
left: 29px; | |||
top: 19px; | |||
border-radius: 0 0.9375rem 0.9375rem 0.9375rem; | |||
border-radius: ${theme.custom.borderRadius}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This breaks the effect of having the top left not having any in the top left. Not sure if this suggestion works, we we could try
border-radius: ${theme.custom.borderRadius}; | |
border-radius: 0 ${theme.custom.borderRadius} ${theme.custom.borderRadius} ${theme.custom.borderRadius}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new design doesn't have that view for the badge. So we can consider it deprecated and keep the new design
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not implementing the new design though, we're in the old, changing border-radius. Now if that's something you've strong opinion about, because you don't like it, I can let it go.
I guess it's ready to merge? |
Now we can merge, got the reply from Cindy about unified border-radius |
Closes #253