feat(blade): chat message component #2513
Conversation
tewarig
requested review from
chaitanyadeorukhkar,
kamleshchandnani,
anuraghazra,
snitin315 and
saurabhdaware
as code owners
🦋 Changeset detectedLatest commit: aa1186d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
✅ PR title follows Conventional Commits specification. |
tewarig
changed the title
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit aa1186d:
|
| ref: React.Ref<BladeElementRef>, | ||
| ): React.ReactElement => { | ||
| const childrenToRender = (): React.ReactElement => { | ||
| // their can be a case where childrens are passed like "{' '} some text" so we need to check if children is string or not |
There was a problem hiding this comment.
already using getStringFromReactText inside childrenToRender, since we expect both string and Card component as children, we need to check if we need to wrap it in Text component
| isError={validationState === 'error'} | ||
| onClick={onClick} | ||
| errorText={errorText} | ||
| children={childrenToRender()} |
There was a problem hiding this comment.
Check once: May cause unnecessary rerenders
There was a problem hiding this comment.
seems fine to me.. refactored a bit
packages/blade/src/components/ChatBubble/SelfMessageBubble.web.tsx
Outdated
Show resolved
Hide resolved
packages/blade/src/components/ChatBubble/SelfMessageBubble.web.tsx
Outdated
Show resolved
Hide resolved
packages/blade/src/components/ChatBubble/__tests__/ChatBubble.web.test.tsx
Outdated
Show resolved
Hide resolved
packages/blade/src/components/ChatBubble/SelfMessageBubble.web.tsx
Outdated
Show resolved
Hide resolved
packages/blade/src/components/ChatBubble/SelfMessageBubble.web.tsx
Outdated
Show resolved
Hide resolved
| cursor={isError ? 'pointer' : 'default'} | ||
| > | ||
| <BaseBox | ||
| maxWidth="240px" |
There was a problem hiding this comment.
is this maxWidth set from design side? Shouldn't this be controlled by the consumer and our chatbubble be fluid to accommodate any UI?
There was a problem hiding this comment.
Yes, maxWidth is set from the design for now. It should be, but currently, Bubble should only be used in specific UIs
There was a problem hiding this comment.
I don't think adding maxWidth is a good idea, we did the same thing with Accordion and it backfired causing a lot of issues because:
Once you add maxWidth removing it in future becomes kind of a styling breakage for consumers. Like now if consumers wants to make Accordion fullWidth they have to explicitly override maxWidth to 100% which isn't intuitive.
It should be fluid and expose maxWidth so consumers can add it.
There was a problem hiding this comment.
It should still have a default maxWidth and then expose to consumers right? otherwise the messages will overflow till the end of chatbox
There was a problem hiding this comment.
checked with design, we can remove maxWidth, allow consumers to pass it. i think instead of having default, message should take what everspace is available
There was a problem hiding this comment.
It should still have a default maxWidth and then expose to consumers right?
No. That's what caused the unintuitive issues that consumers faced.
It should still have a default maxWidth and then expose to consumers right? otherwise the messages will overflow till the end of chatbox
<Box width="500px">
<ChatBubble /> // it's like a block element
</Box>ChatBubble should fluidly take it's parent's width.
| onClick, | ||
| }: DefaultMessageBubbleProps): React.ReactElement => { | ||
| return ( | ||
| <BaseBox maxWidth="296px" onClick={onClick}> |
| onClick, | ||
| }: DefaultMessageBubbleProps): React.ReactElement => { | ||
| return ( | ||
| <BaseBox maxWidth="296px" onClick={onClick}> |
There was a problem hiding this comment.
lets move these px values to common file with token.ts and explain where these values come from? like why is it 296px here and div below is 256px?
There was a problem hiding this comment.
i think we don't need 296px onces. it was for wrapper.
chatMessage have a set width of 240px from design
| <m.div | ||
| style={{ | ||
| display: 'flex', | ||
| }} | ||
| animate={{ rotate: animate ? 90 : 0 }} | ||
| transition={{ | ||
| duration: msToSeconds(theme.motion.duration.gentle), | ||
| repeat: Infinity, | ||
| ease: cssBezierToArray(castWebType(theme.motion.easing.emphasized)), | ||
| delay: msToSeconds(theme.motion.delay.gentle), | ||
| }} |
| cursor={isError ? 'pointer' : 'default'} | ||
| > | ||
| <BaseBox | ||
| maxWidth="240px" |
There was a problem hiding this comment.
Expected for this to have different maxWidth than responder's message?
There was a problem hiding this comment.
yes
for default message -
for self message
There was a problem hiding this comment.
@tewarig can you check with design if this is expected? what is the reason to have 2 different max-widths?
There was a problem hiding this comment.
checked with design . We need two different max-width values, but we are removing max-width from the token. We will allow consumers to pass it instead
| return onClick ? ( | ||
| <ChatMessageButton {...props} onClick={onClick} ref={ref as never}> | ||
| {children} | ||
| </ChatMessageButton> | ||
| ) : ( | ||
| <BaseBox {...props} ref={ref as never}> | ||
| {children} | ||
| </BaseBox> | ||
| ); |
There was a problem hiding this comment.
You can also just do this I believe 🤔
| return onClick ? ( | |
| <ChatMessageButton {...props} onClick={onClick} ref={ref as never}> | |
| {children} | |
| </ChatMessageButton> | |
| ) : ( | |
| <BaseBox {...props} ref={ref as never}> | |
| {children} | |
| </BaseBox> | |
| ); | |
| return ( | |
| <BaseBox as={onClick ? 'button' : undefined} onClick={onClick} {...props} ref={ref as never}> | |
| {children} | |
| </BaseBox> | |
| ); |
| const chatMessageToken = { | ||
| default: { | ||
| maxWidth: '256px', | ||
| }, | ||
| self: { | ||
| maxWidth: '240px', | ||
| padding: 'spacing.4', | ||
| borderTopLeftRadius: 'large', | ||
| borderTopRightRadius: 'large', | ||
| borderBottomLeftRadius: 'large', | ||
| borderBottomRightRadiusForLastMessage: 'none', | ||
| borderBottomRightRadius: 'large', | ||
| backgroundColor: { | ||
| default: 'surface.background.primary.subtle', |
tewarig
force-pushed
the
feat/chat-bubble-component
branch
from
036ecfe to
f3bb245
Compare
|
footerActions are not aligned correctly by default like children. 
|
you can follow this story, https://61c19ee8d3d282003ac1d81c-lbirdazznk.chromatic.com/?path=/story/components-chatmessage--chat-message-with-footer-actions you need to wrap footer actions in box . then you can style them as you want. |
Description
feat: add chat bubble component
Changes
Additional Information
Component Checklist