BUG: Editing of Audio, Video and File Message #697
Conversation
…ideo and file message
|
Instead of showing the error, it's better to hide the option of edit for the audio/ video message. |
|
Yes better |
|
But I think it will be difficult
Not only for audio / video but all types of attachments -> audio, video, file and image |
I think for files , images we can keep the edit option to edit the file description as in RC |
I think this is better |
…ed previous approach of showing popup message
|
Hello Zishan (@Spiral-Memory), I have changed my approach from showing a popup to hiding the 'edit' option for audio/video messages. Thank you, Abir (@abirc8010) and Smriti (@smritidoneria) for your comments. At first, I thought of hiding the 'edit' option for all messages except text, but because of your comments, I understood that this is the better approach. Video: hide.edit.option.for.audio.and.video.messages.mp4 |
| @@ -120,7 +120,7 @@ export const MessageToolbox = ({ | |||
| id: 'edit', | |||
| onClick: () => handleEditMessage(message), | |||
| iconName: 'edit', | |||
| visible: message.u._id === authenticatedUserId, | |||
| visible: (message.u._id === authenticatedUserId) && (message.files?.[0].type !== 'audio/mpeg') && (message.files?.[0].type !== 'video/mp4'), | |||
There was a problem hiding this comment.
But not all audio and video are in MPEG/MP4 format, right?
We might need to create a mapping for suitable formats and then try
There was a problem hiding this comment.
Hello Zishan, Currently all our audios are of type: 'audio/mpeg' & videos are of type: 'video/mp4'
These values are hardcoded in the 'AudioMessageRecorder.js' and 'VideoMessageRecoder.js' files.
So, I think the above condition will work for all audio and video files since we currently have only one format.
If we later introduce new formats for audio and video, we will update this logic accordingly.
There was a problem hiding this comment.
Okay, this is only for the audio and video message recorded directly ?
And not for the file sent ?
|
I think instead of completely removing the edit option , we can disable the audio, video and file option in editing mode like in RC. |
Yes better! |
|
Please resolve the conflict |
Hello Abir, right now I have not disabled edit option for files, user can still edit description for file messages. Currently, our Embedded Chat approach is slightly different for audio and video scenarios than RC. Screenshots from RC- For Video: On Embedded we directly send the audio and video messages, without asking for a description. |
Yes, Zishan. resolving now |
|
Hello Zishan (@Spiral-Memory), resolved the conflict and format check issue. since it's my first PR I apologize for any inconvenience caused by my lack of experience, do give me a review so that I can make better PRs |
|
No worries @dhairyashiil |
| visible: isAllowedToEditMessage, | ||
| visible: | ||
| isAllowedToEditMessage && | ||
| message.files?.[0].type !== 'audio/mpeg' && |
There was a problem hiding this comment.
Instead of checking here directly, make a variable for this check and use it here.
…udio-video-file-message
Brief Title
Updated handleEditMessage functionality, and added validation for audio, video, and file message
When a user sends an audio/video/file message in the chat and clicks on the "Edit" option, they can record another audio or video/upload new file. However, the User should not be able to edit audio or video, or file messages. Instead, they can delete the message and send a new one
Acceptance Criteria fulfillment
Fixes #696
Fixes #698
Screenshots
and
PR Test Details
Note: The PR will be ready for live testing at https://rocketchat.github.io/EmbeddedChat/pulls/pr-697 after approval.