talk-android icon indicating copy to clipboard operation
talk-android copied to clipboard

Refactored Message input view into it's own Fragment

Open rapterjet2004 opened this issue 10 months ago • 10 comments

  • fixes #3784

Introduction

Shouldn't be merged until after the stable release.

MessageInput is very complex, spanning 1300+ lines of code. It deals with sending, recording, emojis, files, editing, mentioning, etc. It should be placed into it's own fragment, so ChatActivity becomes easier to maintain in the long run. Bit of an investment cost though and not necessarily a priority in case other features require attention. But it's something that should be taken care of before it gets out of control. Chatting is the core functionality of talk-android, and needs to be structured well.

This also gives me an opportunity to clean up the architecture of the view. Separate UI from business logic, and deal with asynchronous work in a less "Spaghetti" manner. Less threads, more coroutines. Less logic in the UI, more logic in the model layer, etc

The plan is the separate the voice recording from the typical messaging. To have two different fragments to handle the behavior. A significant amount of code (admittedly my own 😅 ) is involved in the voice recording logic, which can be pretty complicated. Fragments transaction events will be observed by the ChatViewModel and handled.

As a rule of thumb, I'm going for 25/15/60 (view, view model, model) when implementing logic in MVVM. Not everything is implemented in MVVM, just the functionality that relate to message input (with the exception of typing status).

PR Diagram

Arrows and colors represent the flow and scope of information traveling down from user input. Nextcloud MessageInput Diagram (6)

// TODO normal message input

  • [x] sending messages
  • [x] sending attachments
  • [x] emoji button
  • [x] replying
  • [x] editing
  • [x] mentioning
  • [x] typing status?
  • [x] recording

// TODO voice message input

  • [x] Abstracting over MediaRecorder (Note to self: Make mediarecorder state livedata and immutable, good design)
  • [x] Abstract over MediaPlayer
  • [x] Abstract over AudioRecord
  • [x] MicInputCloud
  • [x] previewing // note pause mediaplayer before seeking and resume after
    • [x] Preview SeekBar
    • [x] pause/Play
  • [x] sending preview
  • [x] deleting preview
  • [x] Implement Audio Focus Request (perhaps make this a manager too?)
    • [x] Media Player ( in ChatActivity )
    • [x] Voice Message preview player
    • [x] Media Recorder
    • [x] MicInput

// TODO clean up

  • [x] Delete all the old code
  • [x] Bugs
    • [x] save messageInputView text and cursor state
    • [x] save position and voicemessage on state change in saved ( this is an issue with the lifecycle being recreated )
    • [x] checkShowMessageInputView
    • [x] checkLobbyState
    • [x] onCreateOptionsMenu (used for context???)
    • [x] handleOnBackPressed
    • [x] Make PreviewSeekbar seekable
    • [x] Fix Voice record bugs
    • [x] use upload file from chatView
    • [x] Micinput restart ui
    • [x] If the preview playback is interrupted, the seekbar will not seek on following recordings, might be related to AudioFocusRequest
    • [x] AudioFocus crash, need to handle receiver illegal arg exception

🏁 Checklist

  • [ ] ⛑️ Tests (unit and/or integration) are included or not needed
  • [ ] 🔖 Capability is checked or not needed
  • [ ] 🔙 Backport requests are created or not needed: /backport to stable-xx.x
  • [ ] 📅 Milestone is set
  • [x] 🌸 PR title is meaningful (if it should be in the changelog: is it meaningful to users?)

rapterjet2004 avatar Apr 01 '24 16:04 rapterjet2004