feat: async voice messages recording #2339
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # src/context/ComponentContext.tsx
myandrienko
reviewed
Mar 22, 2024
| <button | ||
| className='str-chat__attachment-preview-delete' | ||
| data-testid='file-preview-item-delete-button' | ||
| disabled={attachment.$internal?.uploadState === AttachmentUploadState.UPLOADING} |
There was a problem hiding this comment.
Can you explain this $internal stuff? Is it something voice message related? I haven't seen it for attachments before.
There was a problem hiding this comment.
This are internal metadata that are not shared with the back-end. I decided to keep them "isolated" under this key and just remove them before sending the message with the attachment to the back-end. Current implementation with image and file uploads seems to me clunky in that it does not isolate these locally important metadata and even use different names for those attributes, that are later removed before the submission.
I would like to propose this approach also to other types of attachments gradually to make our code more readable.
There was a problem hiding this comment.
I see, here's what worries me: integrators implementing custom attachment previews shouldn't care about what is and what isn't part of our backend schema. But in this case they will have to use this $internal property, and they may find it a bit weird.
src/components/MessageInput/types.ts
Outdated
| @@ -3,6 +3,12 @@ import type { DefaultStreamChatGenerics } from '../../types/types'; | |||
|
|
|||
| type AttachmentLoadingState = 'uploading' | 'finished' | 'failed'; | |||
|
|
|||
| export enum AttachmentUploadState { | |||
There was a problem hiding this comment.
Is this different from UploadState?
There was a problem hiding this comment.
I wanted to gradually replace the union type with enum that could be used by the integrators too - if they want to do some customizations. Removing the original would be breaking. WDYT, how could this be approached?
There was a problem hiding this comment.
I actually don't know, the union type doesn't sound bad, can we just keep using the UploadState union in this case? Anyway, I don't have any strong opinion here.
|
@myandrienko thank you for the review. I forgot to put this into draft state when creating this PR. I plan to clean up the stuff still 🙂 |
|
Size Change: +305 kB (+17%) Total Size: 2.12 MB
ℹ️ View Unchanged
|
oliverlaz
reviewed
Apr 11, 2024
src/components/MediaRecorder/classes/MediaRecorderController.ts
Outdated
Show resolved
Hide resolved
src/components/MediaRecorder/classes/MediaRecorderController.ts
Outdated
Show resolved
Hide resolved
| * @param file | ||
| */ | ||
| export const toAudioBuffer = async (file: File) => { | ||
| const audioCtx = new AudioContext(); |
There was a problem hiding this comment.
Should we close this context eventually? Does this create a leak?
There was a problem hiding this comment.
Fixed by closing the context: dab90e2
# Conflicts: # src/components/Attachment/__tests__/Audio.test.js # src/i18n/de.json # src/i18n/en.json # src/i18n/es.json # src/i18n/fr.json # src/i18n/hi.json # src/i18n/it.json # src/i18n/ja.json # src/i18n/ko.json # src/i18n/nl.json # src/i18n/pt.json # src/i18n/ru.json # src/i18n/tr.json
oliverlaz
reviewed
Apr 17, 2024
docusaurus/docs/React/components/message-input-components/audio-recorder.mdx
Outdated
Show resolved
Hide resolved
docusaurus/docs/React/components/message-input-components/audio-recorder.mdx
Outdated
Show resolved
Hide resolved
oliverlaz
approved these changes
Apr 17, 2024
|
🎉 This PR is included in version 11.15.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🎯 Goal
Introduce
🛠 Implementation details
Provide a description of the implementation
🎨 UI Changes
Please see the attached documentation files.