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

Voice recording UI state in ViewModel #4515

Merged
merged 17 commits into from Nov 19, 2021
Merged

Voice recording UI state in ViewModel #4515

merged 17 commits into from Nov 19, 2021

Conversation

ouchadam
Copy link
Contributor

Inverts the VoiceMessageRecorderView update flow so that TextComposerViewModel controls the RecordingUiState

  • Allows the dragging to cancel or lock mid way, the current behaviour forces whichever action the user starts dragging towards
  • Fixes a small UI quirk where the recording layout would jump, caused by aligning to the mic button rather than the send button
  • Fixes the dragging becoming inverted if dragging past the initial touch position
  • Tentatively fixes Unable to record a voice message after pressing the microphone icon a few times #4400 (I've not been able to reproduce on this branch at least)

Next step is to port the Voice draft to the SDK #4237

CANCEL LOCK SEND LOCK DELETE
after-voice-cancel after-lock-send after-voice-lock-delete
CANCEL OR LOCK HOLDING
after-cancel-or-lock after-voice-hold

@github-actions
Copy link

github-actions bot commented Nov 18, 2021

Unit Test Results

  66 files  ±0    66 suites  ±0   1m 6s ⏱️ -6s
135 tests ±0  135 ✔️ ±0  0 💤 ±0  0 ±0 
418 runs  ±0  418 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 7d0d105. ± Comparison against base commit d00277a.

♻️ This comment has been updated with latest results.

@DoM1niC
Copy link

DoM1niC commented Nov 18, 2021

Nice One :) can you try to fix the multiple send issue if you send more than one Voice Message the following Messages will Stuck and can't be resended.

Update
Ahhh sry I see this will important for Draft SDK Stuff #4237

@ouchadam
Copy link
Contributor Author

@DoM1niC I'll have a look since I'm in the area!

) : MavericksState {

val isVoiceRecording = when (voiceRecordingUiState) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the isRecording and isIdle states are inferred from the UI state

import im.vector.app.features.home.room.detail.composer.voice.VoiceMessageRecorderView.RecordingUiState
import im.vector.app.features.home.room.detail.timeline.helper.VoiceMessagePlaybackTracker

class VoiceMessageViews(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

all of the android widget interactions have been extracted into this delegate, to help keep the VoiceMessageRecorderView as simple as possible

…which actions should be handled in each layer
@@ -0,0 +1 @@
Voice recording mic button refactor with small animation tweaks in preparation for voice drafts
Copy link
Contributor Author

Choose a reason for hiding this comment

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

wasn't 100% sure if the change log entry was needed as this is mostly refactoring (same as #4523)

Copy link
Member

Choose a reason for hiding this comment

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

Always OK to have a changelog, if something goes wrong (I hope no!), we keep quickly see in the change history what have been done.

Copy link
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

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

LGTM, small remark on naming.
Did not test, will rather test after change on draft handling 👍

voiceMessageViews.renderVisibilityChanged(parentChanged, visibility)
}

fun display(recordingState: RecordingUiState) {
Copy link
Member

Choose a reason for hiding this comment

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

Nit: in the codebase those methods are named render()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, will update in the other PR

@@ -0,0 +1 @@
Voice recording mic button refactor with small animation tweaks in preparation for voice drafts
Copy link
Member

Choose a reason for hiding this comment

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

Always OK to have a changelog, if something goes wrong (I hope no!), we keep quickly see in the change history what have been done.

@bmarty bmarty merged commit 35f9bef into develop Nov 19, 2021
@bmarty bmarty deleted the feature/adm/voice-state branch November 19, 2021 14:59
@bmarty
Copy link
Member

bmarty commented Nov 19, 2021

Stop the CI, due to add changelog entry. Previous commit was green (sort of)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to record a voice message after pressing the microphone icon a few times
3 participants