-
Notifications
You must be signed in to change notification settings - Fork 150
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
feat: chat bubble api decision #2508
base: master
Are you sure you want to change the base?
Changes from 21 commits
8115801
e09efbe
e0e15fc
98ecf95
8ca3a6d
55d79b1
2c7cc51
f18bd9d
f0bf083
30c0443
422235b
21653d5
ad47255
a700c3a
5644194
37b9e5f
b9a36fe
77bdcf7
e3ab4cb
36cda0b
fecd872
e3ce385
b5f901a
8a36c0b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
# ChatMessage Decisions | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can add annotations image from figma cover of this component like we do in other API decisions |
||
- [Design](#design) | ||
- [ChatMessage Component](#ChatMessage-component) | ||
- [ChatMessage API](#ChatMessage-api) | ||
- [Open Questions](#open-questions) | ||
|
||
## Design | ||
|
||
[Figma Link](https://www.figma.com/design/jubmQL9Z8V7881ayUD95ps/Blade-DSL?node-id=100413-32686&t=n9A7LztwEkIsly3v-0) | ||
|
||
## ChatMessage Component | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We usually keep Usage section first to give idea of the API and then add props table and then more examples of usage |
||
|
||
This will be our main component that will be used to render the chat bubble. | ||
|
||
## ChatMessage API | ||
|
||
| Prop | Type | Default | Required | Description | | ||
| ---------------------- | ------------------------------------------- | ------- | -------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| messageType | 'last' or 'default' | default | No | If the message is the last message in the chat and if this prop is enabled we will add decorations messageBubble | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The description of this prop is not clear, what does decorations messageBubble mean? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. update it to - |
||
| senderType | 'self' or 'other' | self | No | we will add different styles based on this bubble | | ||
| isLoading | Boolean | false | No | If the message is loading, we will add a loading animation to the chat bubble | | ||
| validationState | 'error' or 'none' | none | No | if validation state is error we will show error decoration and message| | ||
| errorText | String | null | No | If the message is an error message, we will show the error message in the chat bubble | | ||
| onClick | Function | null | No | callback to be called when ever component is clicked. | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it need to be interactive/clickable? If we render a Card inside the slot, it would support its own There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, in case of an error, we need to support the onClick prop since children accepts JSX. It should support its own onClick, but in that case, we need to stop event bubbling. |
||
| footerActions | ReactNode | null | No | if this is passed we will render this at the end of chat bubble | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a sample usage for this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added -
|
||
| children | 'ReactNode' or 'string' | null | yes | The children that will be rendered inside the chat bubble. can be react node or a string | | ||
| leading | ReactNode | null | No | will be displayed only in case if message is other and also, will contain animation by default | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need a separate prop for just controlling typing animation as well, because otherwise if the component re-mounts/re-renders, the text will animate once again. Screen.Recording.2025-01-31.at.7.29.29.PM.movThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. typing animation will be controlled by user. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We would just be adding an animation to the leading. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated to - |
||
|loadingText | String | null | No | if loading is true, we will show this text in the chat bubble | | ||
|
||
```tsx | ||
type ChatMessageProps = { | ||
messageType?: 'last' | 'default'; | ||
senderType?: 'self' | 'other'; | ||
isLoading?: boolean; | ||
validationState?: 'error' | 'none'; | ||
errorText?: string; | ||
onClick?: () => void; | ||
footerActions?: ReactNode; | ||
children: ReactNode | string; | ||
leading?: ReactNode; | ||
loadingText?: string; | ||
}; | ||
``` | ||
|
||
## API | ||
|
||
```tsx | ||
|
||
// for animation | ||
<Move> | ||
<ChatMessage senderType="self">Demo Text</ChatMessage> | ||
</Move> | ||
|
||
// with error | ||
<ChatMessage validationState="error" errorText="Error Message">Demo Text</ChatMessage> | ||
|
||
// with card | ||
<ChatMessage><Card></Card></ChatMessage> | ||
|
||
Comment on lines
+55
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about this API? // normal string text
<ChatMessage>string message</ChatMessage>
// With a complex body like Mardown/Card
<ChatMessage>
<ChatMessageBody>
<Markdown> Demo Text </Markdown>
<Card></Card>
</ChatMessageBody>
<ChatMessageFooter>
<Button> Get STarted</Button>
</ChatMessageFooter>
</ChatMessage> There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the Compound Component Pattern would make more sense if we had shared state between the child components. |
||
|
||
<ChatMessage><Markdown> Demo Text </Markdown></ChatMessage> | ||
<ChatMessage><Markdown> Demo Text </Markdown></ChatMessage> | ||
``` | ||
|
||
## Alternative API | ||
|
||
```tsx | ||
// for animation | ||
<Move> | ||
<ChatMessage message="Demo Text"/> | ||
</Move> | ||
|
||
// with card | ||
<ChatMessage cardBody={<SomeComponent/>} /> | ||
|
||
|
||
|
||
// Markdown | ||
<ChatMessage markdown="# this is markdown" /> | ||
``` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can later add pros and cons and why we went with that children API |
||
|
||
## Open Questions | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So normally we keep the questions in open questions and add conclusion and discussion points based on discussion so that we have reasoning in place if we want to find it later |
||
- should their be an animation in case of error? |
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.
You can get the format from other API decision