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

[Story] Text composer draft #1776

Closed
13 tasks done
manuroe opened this issue Jun 10, 2023 · 8 comments
Closed
13 tasks done

[Story] Text composer draft #1776

manuroe opened this issue Jun 10, 2023 · 8 comments
Assignees
Labels
A-Composer App: ElementX Android App: ElementX iOS Rust: Easy SDK tasks should be doable by any developer like FFI export or small behavior changes Team: Element X Platform

Comments

@manuroe
Copy link
Member

manuroe commented Jun 10, 2023

  • 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

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

Subtasks

Android

  1. A-Draft T-Task

iOS

  1. T-Task
    Velin92

QA feedback

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 manuroe added Rust: Easy SDK tasks should be doable by any developer like FFI export or small behavior changes and removed Rust: Expert Some tasks can only be implemented by SDK experts labels Apr 10, 2024
@pixlwave
Copy link
Member

pixlwave commented May 3, 2024

Related regarding behaviour when editing: element-hq/element-x-ios#2759

@Velin92
Copy link
Member

Velin92 commented May 23, 2024

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?

@manuroe
Copy link
Member Author

manuroe commented May 24, 2024

@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
Copy link
Member

bnjbvr commented May 24, 2024

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
Copy link
Member

Velin92 commented Jun 12, 2024

I also addd an API here: matrix-org/matrix-rust-sdk#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
Copy link
Member

Velin92 commented Jun 12, 2024

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:
matrix-org/matrix-rust-sdk#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
Copy link
Member Author

manuroe commented Jun 12, 2024

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
Copy link
Member Author

manuroe commented Sep 11, 2024

We did everything we wanted.

@manuroe manuroe closed this as completed Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Composer App: ElementX Android App: ElementX iOS Rust: Easy SDK tasks should be doable by any developer like FFI export or small behavior changes Team: Element X Platform
Projects
None yet
Development

No branches or pull requests

5 participants