-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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(@aws-amplify/ui-components): Add ChatBot Component #6563
Conversation
Codecov Report
@@ Coverage Diff @@
## ui-components/main #6563 +/- ##
===================================================
Coverage 73.30% 73.30%
===================================================
Files 208 208
Lines 12928 12928
Branches 2433 2433
===================================================
Hits 9477 9477
Misses 3288 3288
Partials 163 163
Continue to review full report at Codecov.
|
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.
Some nits / non blocking suggestions.
packages/amplify-ui-components/src/common/audio-control/helper.ts
Outdated
Show resolved
Hide resolved
packages/amplify-ui-components/src/components/amplify-chatbot/amplify-chatbot.tsx
Outdated
Show resolved
Hide resolved
/** | ||
* Interactions helpers | ||
*/ | ||
private async sendTextMessage() { |
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.
Could we not move this to a separate interactions-helpers
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.
hmmmm that's a good idea; I thought about it too, but I decided not to for now because both sendText
and sendVoice
have setState logic throughout them. I'll see how interactions-helpers would look after the preview release.
packages/amplify-ui-components/src/components/amplify-chatbot/amplify-chatbot.tsx
Outdated
Show resolved
Hide resolved
packages/amplify-ui-components/src/common/audio-control/recorder.ts
Outdated
Show resolved
Hide resolved
packages/amplify-ui-components/src/common/audio-control/recorder.ts
Outdated
Show resolved
Hide resolved
@wlee221 Looks awesome 👏 Great work 🎉 🌮 Wondering if you already tested this cross browsers to see how UI is rendered and if all functionalities work across different browsers.? |
Co-authored-by: Ashika <35131273+ashika01@users.noreply.github.com>
This pull request introduces 1 alert and fixes 1 when merging 0acd8f8 into 00d5e17 - view on LGTM.com new alerts:
fixed alerts:
|
…s-amplify/amplify-js into ui-components/chatbot-staging
Thanks Ashika! I tested this on Firefox / Chrome with React and Vue. I'll test for Angular and safari. |
This pull request introduces 1 alert and fixes 1 when merging 378e921 into 00d5e17 - view on LGTM.com new alerts:
fixed alerts:
|
Just tested for safari and it doesn't support promise based |
This pull request introduces 1 alert and fixes 1 when merging 212d4a5 into 46770e9 - view on LGTM.com new alerts:
fixed alerts:
|
This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs. Looking for a help forum? We recommend joining the Amplify Community Discord server |
Issue #, if available:
Description of changes: This PR adds
amplify-chatbot
as part of the Amplify UI Component redesign.Features:
Interactions.send
amplify-toast
Customization:
Previous Components Improvements:
amplify-button
amplify-button
,amplify-icon
, andamplify-input
.amplify-icon
.span
insideamplify-icon
.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.