Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Limit voice recording length #5871

Merged
merged 8 commits into from
Apr 19, 2021
Merged

Limit voice recording length #5871

merged 8 commits into from
Apr 19, 2021

Conversation

turt2live
Copy link
Member

@turt2live turt2live commented Apr 15, 2021

Requires #5870


Reviewer:

  • The real diff is at pull/5870/head...pull/5871/head
  • This is reviewable commit-by-commit (recommended for context).
  • This intentionally does not implement the UI for "you stopped recording" - this is punted to a future PR.

image

See diff for details. Note that this introduces an "Uploading" state which is not currently used.

At the moment, if a user hits the maximum time then their recording will be broken. This is expected to be fixed in a future PR.
This makes it easier to keep track of which pieces the client will have already dispatched or been executed, reducing the amount of class members needed.

Critically, this makes it so the 'stop' button (which is currently a send button) actually works even after the automatic stop has happened.

UI is still pending for stopping recording early. This is not covered by this change.
@turt2live turt2live requested a review from a team April 15, 2021 04:07
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

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

Overall looks good, just a few tweaks here and below.

src/components/views/rooms/MessageComposer.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

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

Gathered these comments as a review... 😅

src/utils/Singleflight.ts Show resolved Hide resolved
src/utils/Singleflight.ts Show resolved Hide resolved
test/Singleflight-test.ts Show resolved Hide resolved
@turt2live
Copy link
Member Author

@jryans please take a look - should be able to use the regular diff view now

@turt2live turt2live requested a review from jryans April 16, 2021 16:12
Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

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

A tiny tweak, but anyway looks good to merge without another round, thanks! 😄

src/utils/Singleflight.ts Outdated Show resolved Hide resolved
@turt2live turt2live merged commit 01fc88f into develop Apr 19, 2021
@turt2live turt2live deleted the travis/voice/countdown branch April 19, 2021 16:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants