-
Notifications
You must be signed in to change notification settings - Fork 55
feat(ChatMessage): add badge and badgePosition props #823
Conversation
-restricted position to start and end
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.
I am not sure about badge
, can we go with label
for now?
And when we will have decision about naming just rename it everywhere?
# Conflicts: # CHANGELOG.md
Codecov Report
@@ Coverage Diff @@
## master #823 +/- ##
=======================================
Coverage 93.54% 93.54%
=======================================
Files 21 21
Lines 728 728
Branches 69 69
=======================================
Hits 681 681
Misses 47 47 Continue to review full report at Codecov.
|
@layershifter in my opinion label as a word has nothing semantically with the ChatMessage, although it is shorthand for label (yep, the name had already produced us enough headache). Even if this was shorthand for the Icon component, I would still go with badge, as it is much more clear what it represents... |
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.
What about Styled Chat Item example?
return { | ||
boxShadow: v.badgeShadow, | ||
position: 'absolute', | ||
height: pxToRem(24), |
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 when you change icon size. Any chance this could work without absolute values?
Something like:
boxShadow: v.badgeShadow,
position: 'absolute',
padding: pxToRem(4),
height: 'auto',
width: 'auto',
borderRadius: '50%',
top: pxToRem(4),
zIndex: '1',
[sidePosition]: 0,
transform: p.badgePosition === 'start' ? 'translateX(-50%)' : 'translateX(50%)',
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.
+1 for using transform
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 Miro, I've changed the styles, work with every size of icon, and the default is according to the specs. I've added badge in the styles ChatItem example as well. Let me know if there is anything else.
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.
looks good, I vote for having badge
for name for now
https://ant.design/components/badge/
-improved styles for the badge
@@ -83,6 +93,7 @@ class ChatMessage extends UIComponent<ReactProps<ChatMessageProps>, ChatMessageS | |||
static defaultProps = { | |||
accessibility: chatMessageBehavior, | |||
as: 'div', | |||
badgePosition: 'end', |
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.
not sure about this default - lot of examples where badge appears at start
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.
not sure about default badge's position - I would rather suggest to make it start
one.
This PR adds the badge and badgePosition ('start' | 'end') props to the ChatMessage.
TODO
Here are examples of this:
Example in chat: