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

Add voice message rendering to the timeline #1776

Merged
merged 10 commits into from
Sep 21, 2023

Conversation

nimau
Copy link
Contributor

@nimau nimau commented Sep 21, 2023

This PR adds:

  • voice message rendering into the timeline (part of the Voice Messages feature)
  • a feature flag voiceMessageEnabled

The LongPressViewFeedback modifier has been slightly updated to allow the long press action to be disabled for a specific timeline item, so the drag gesture on the waveform can be performed without triggering the standard long press gesture action.

Notes:
The VoiceRoomPlaybackView observes an instance of the VoiceRoomPlaybackViewState which is managed by the TimelineController. This VoiceRoomPlaybackViewState instance will be used to monitor the state of the audio player, and to restore its state (progress for example) when it is attached. This should allow us to have just one instantiated audio player for the whole timeline.
All actions triggered by the VoiceRoomTimelineView are sent back to the RoomTimelineController, which will decide whether or not to attach an audio player (and restore the the playback state if necessary). In this first PR, the audio playback is not implemented.

@nimau nimau requested a review from a team as a code owner September 21, 2023 10:06
@nimau nimau requested review from Velin92 and removed request for a team September 21, 2023 10:06
@github-actions
Copy link

github-actions bot commented Sep 21, 2023

Warnings
⚠️ This pull request seems relatively large. Please consider splitting it into multiple smaller ones.
⚠️ Please add a changelog.
⚠️ Some of the commits are missing ticket numbers. Please consider squashing all commits that don't have a tracking number.
⚠️ You seem to have made changes to views. Please consider adding screenshots.

Generated by 🚫 Danger Swift against b617ee9

Copy link
Member

@Velin92 Velin92 left a comment

Choose a reason for hiding this comment

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

LGTM, however I would check if the audio message supports replies and thread decorators correctly, also need to add all the screens to the preview tests

@nimau nimau force-pushed the nicolas/voice-messages-timeline-rendering branch from 004d070 to b617ee9 Compare September 21, 2023 14:08
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

No Coverage information No Coverage information
5.2% 5.2% Duplication

@nimau nimau enabled auto-merge (squash) September 21, 2023 14:19
@nimau nimau merged commit b8e5611 into develop Sep 21, 2023
@nimau nimau deleted the nicolas/voice-messages-timeline-rendering branch September 21, 2023 14:36
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.

2 participants