search

element-hq / element-meta

Shared/meta documentation and project artefacts for Element clients
74 stars 12 forks source link

[Story] Text composer draft #1776

Closed manuroe closed 1 month ago

manuroe commented 1 year ago

Acceptance criteria

For editing & reply

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.

  2. 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

Size estimate

None

Dependencies

Out of scope

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
- [x] 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

iOS

pixlwave commented 5 months ago

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

Velin92 commented 4 months ago

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

manuroe commented 4 months ago

@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"?

bnjbvr commented 4 months ago

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.

Velin92 commented 4 months ago

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 commented 4 months ago

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

manuroe commented 4 months ago

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 commented 1 month ago

We did everything we wanted.