Skip to content
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

[MM-57365] Stop recording confirmation modal #760

Merged
merged 5 commits into from
Jun 12, 2024

Conversation

cpoile
Copy link
Member

@cpoile cpoile commented May 31, 2024

@cpoile cpoile added 1: UX Review Requires review by ux 2: Dev Review Requires review by a core committer labels May 31, 2024
@cpoile cpoile added this to the v0.28.0 / MM 9.10 milestone May 31, 2024
@streamer45
Copy link
Collaborator

I assume this is by design, so it's more of a question for @abhijit-singh. I don't see any mention of transcriptions, which is a bit inconsistent with the rest of the related messages.

Copy link
Collaborator

@streamer45 streamer45 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Nothing blocking, but I noticed the stop button is not strictly following the latest updates on similar "destructive" buttons in terms of hover and active states and related background/text colors.

@cpoile
Copy link
Member Author

cpoile commented Jun 6, 2024

Thanks! Fixed the background.

@cpoile cpoile removed the 2: Dev Review Requires review by a core committer label Jun 6, 2024
Copy link

@abhijit-singh abhijit-singh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome @cpoile! Thank you!

For the case where transcription is also enabled, we can have the modal say :
Title: Stop recording and transcription
Body: The call recording and transcription files will be processed and posted in the call thread. Are you sure you want to stop the recording and transcription?

image

cc @cwarnermm

@cpoile
Copy link
Member Author

cpoile commented Jun 11, 2024

Thanks @abhijit-singh, updated with the new text.

@cpoile cpoile added 3: Reviews Complete All reviewers have approved the pull request and removed 1: UX Review Requires review by ux labels Jun 11, 2024
@cpoile cpoile merged commit 2a7b5a3 into main Jun 12, 2024
20 checks passed
@cpoile cpoile deleted the MM-57365-stop-recording-confirm branch June 12, 2024 12:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants