-
Notifications
You must be signed in to change notification settings - Fork 310
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: bot names and avatars #3071
Conversation
sets up a proivder model for getting Bot based data refactor to create a base class for stateful clients creates a new component for rendering chat avaatars that can handle bots fix: stop creating two instances of the notification client closes #3068 fix: remove contenteditable attribute from emjoi html closes #3060
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 have a few comments, but I will test this out this afternoon as well and see if I find anything.
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 would like to see the ChatHeader have the bot name when it is a 1:1 chat with a bot like Teams.
That's in the nice to have category, tracking this as #3084 |
That makes sense to me, I suspect we'll have to do something here to infer the avatar like exposing a map keyed on the chatId returning a list of Bots on the state emitted by BotInfoClient as I'm not sure that we can rely on the installedApps collection to reflect the Bots that are active in the conversation. |
🚀 New react-contoso sample application deployed here |
|
🚀 New react-contoso sample application deployed here |
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.
lgtm
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 to me.
Closes #2947
Closes #3060
Closes #3068
PR Type
Description of the changes
sets up a React Context + Provider model for getting Bot based data via a shared Stateful Client
refactor to create a base class for stateful clients
creates a new component for rendering chat avatars that can handle bots
stop creating two instances of the notification client
remove contenteditable attribute from emjoi html
PR checklist
yarn build
) and changes have been tested in at least two supported browsers (Edge + non-Chromium based browser)yarn setLicense
)Other information