-
Notifications
You must be signed in to change notification settings - Fork 756
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
Conversation
- creates a display entry point which will be called externally
- fixes other quirks caused by porting to the inverted display logic
… the margin, fixes the layout jumping when the mic button switches to a send
…the file straight away
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 |
@DoM1niC I'll have a look since I'm in the area! |
) : MavericksState { | ||
|
||
val isVoiceRecording = when (voiceRecordingUiState) { |
There was a problem hiding this comment.
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( |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
There was a problem hiding this 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) { |
There was a problem hiding this comment.
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()
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
Stop the CI, due to add changelog entry. Previous commit was green (sort of) |
Inverts the
VoiceMessageRecorderView
update flow so thatTextComposerViewModel
controls theRecordingUiState
Next step is to port the Voice draft to the SDK #4237