element-meta icon indicating copy to clipboard operation
element-meta copied to clipboard

[Story] Text composer draft

Open manuroe opened this issue 2 years ago • 7 comments

  • As a user
  • I want to keep text I typed in the text composer but not yet sent when I switch rooms
  • So that I do not have to type the same text again

Acceptance criteria

  • Any message that is using the composer-input field should be persisted per timeline: Text messages including formatting (example: if the user made something bold - it should still be bold), editing and reply.
  • Store when you leave the room or background the app
  • Draft messages must survive an app kill
  • Draft messages must be stored encrypted
  • TBC with expected behavior for draft on a reply and on edit
  • If the app crashes it is ok if the draft could not be saved

For editing & reply

  • Just persits the message as it was at the time the user hit “edit” or reply.
  • You do not need to take into consideration if the message has been edited in the meantime. The user can just send the reply.
  • If the message to reply (edited) has been deleted in the meantime just show an error. Fail gracefully :)

In case that we have trouble sending the message (edit or reply) because it has not been loaded into the timeline

The main objective is to send the message.

This means in for prios:

  1. Make sure replies work
  • The user expectation would be that the reply state is persisted and the message is send.
  • If you cannot do that: send the message w.o. the reference to the replied message.
  1. Edits
  • The user expectation would be that the edit state is persisted and the message is send.
  • If you cannot do that: show an error

PRD https://docs.google.com/document/d/1c8je627qaT1FCh1qHsnFM1u7wsdNSkLmqLIW3xExLd0/edit

Leads

  • Tech: @Velin92
  • Design: @amshakal

Size estimate

None

Dependencies

  • None

Out of scope

  • Low prio bug: RTE: Formatting with quote can be lost when restoring the draft (see our internal doc for steps to reproduce)

Open questions

### Questions
- [x] ~~Should we store draft for editing?~~ Yes
- [x] ~~Should we store draft for a reply differently than a normal message? Idea: when coming back to the room, we could resume the reply UI if there is a reply draft~~ we will store the state of the composer if is a reply with the event ID is pointing to.

Subtasks

### Android
- [ ] https://github.com/element-hq/element-x-android/issues/2869
### iOS
- [ ] https://github.com/element-hq/element-x-ios/issues/2849
### Rust
- [ ] https://github.com/matrix-org/matrix-rust-sdk/issues/3430
- [ ] https://github.com/matrix-org/matrix-rust-sdk/pull/3534
- [ ] https://github.com/matrix-org/matrix-rust-sdk/issues/3538
### QA feedback
- [x] Android: if the user decides to edit an existing message while they were typing a new message in the composer, we must restore the new message draft after the editing. We can store it in memory
- [x] iOS: if the user decides to edit an existing message while they were typing a new message in the composer, we must restore the new message draft after the editing. We can store it in memory
- [x] iOS: Sending a draft of an edit does not update the message in the timeline (see our internal doc for steps to reproduce)
- [x] (low prio) RTE: Formatting with quote can be lost when restoring the draft (see our internal doc for steps to reproduce) --> Moved to out of scope
- [ ] https://github.com/element-hq/element-x-ios-rageshakes/issues/1948
- [ ] Similar to the above iOS issue, Android does not lose the mentioned users but they reappear as MD in the composer instead of pills

Sign-off

Android

  • [ ] Design sign-off on completion
  • [ ] QA sign-off on completion
  • [ ] Product sign-off on completion

iOS

  • [ ] Design sign-off on completion
  • [ ] QA sign-off on completion
  • [ ] Product sign-off on completion

manuroe avatar Jun 10 '23 10:06 manuroe

Related regarding behaviour when editing: https://github.com/element-hq/element-x-ios/issues/2759

pixlwave avatar May 03 '24 10:05 pixlwave

I made a lot of progress today and I am able to store and restore messages, and also edit and reply states, I am even able to asynchronously loading reply details of an item that has not been paginated in the timeline, and even display a loading state in the meantime

  • The set html function for the composer seems to be broken, I even tested it for normal edits and seems to not be able to parse the formatting properly, maybe it was caused by the recent changes in the composer behaviour? @stefanceriu
  • Restoring edits and replies works great, and I even implemented a way to get those details async, but the problem is that since those events MAY NOT BE PAGINATED in the timeline, when the event is sent it fails, since we use the timeline APIs for sending messages, replies and edits, and the timeline item they are referencing in the latter two cases, is not found by the timeline itself. @bnjbvr
  • The InReplyToDetails seem to not contain information if they are part of a thread or not, but maybe is just a matter of exposing a boolean API on them?

Velin92 avatar May 23 '24 21:05 Velin92

@VolkerJunginger how do you want to manage the edge case described in the second point about "Restoring edits and replies for messages no more in the cache"?

manuroe avatar May 24 '24 08:05 manuroe

We've discussed the issue with Mauro, and I don't think there's a technical reason the replied-to / updated event has to be in the timeline, since the event could be fetched based on its ID, if it's missing from the timeline, and a synthetic timeline item could be created from that — let's see if that technical solution is fruitful before deciding what to do at a product-wide level.

bnjbvr avatar May 24 '24 08:05 bnjbvr

I also addd an API here: https://github.com/matrix-org/matrix-rust-sdk/pull/3534 that allows to async load a reply for an event that has not been paginated yet, this is useful to load the reply preview in the text composer when the draft is restored, and such event is very old, or has not been loaded yet in the timeline.

Velin92 avatar Jun 12 '24 16:06 Velin92

After pairing with @bnjbvr we agreed to fist implement the APIs required to make the draft work and then solve the issue we found caused by the fact that edit and replies won't work for items that have not been paginated yet: https://github.com/matrix-org/matrix-rust-sdk/issues/3538

We will handle this as a separate issue and in a separate PR In the meantime since the rest of the SDK work is done we can unblock the mobile apps and start merging the draft as a first iteration. Should we feature flag it? or can we keep it as it is even with the current API limitation?

@manuroe

Velin92 avatar Jun 12 '24 16:06 Velin92

If we can have the next iteration before Tuesday, our next RC, it is fine to merge it without a feature flag. End users will see it as a bug and we do not want to ship bugs in production.

As you suggested on matrix, @Velin92 , we can merge it now without a FF. If it appears we cannot make the next iteration before Tuesday, we will add a FF to hide from production users.

manuroe avatar Jun 12 '24 16:06 manuroe

We did everything we wanted.

manuroe avatar Sep 11 '24 12:09 manuroe