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] Local echo for media upload #2549

Closed
6 tasks done
manuroe opened this issue Oct 1, 2024 · 6 comments
Closed
6 tasks done

[Story] Local echo for media upload #2549

manuroe opened this issue Oct 1, 2024 · 6 comments

Comments

@manuroe
Copy link
Member

manuroe commented Oct 1, 2024

Description

  • As a user
  • I want to see a local echo of my media uploads in the timeline while they are still being uploaded
  • So that I can continue using the app without being blocked by the upload progress screen as we have now

Definition

Media: Any file currently sent by the app: image, a video or file of any type.

Acceptance criteria

  • Uploading media no longer results in a blocking UI while the upload is in progress
  • The local echo shows a thumbnail of the media being uploaded
  • The upload state is clear for the sender
  • Retries in case of failure are managed like for the message sending queue: except if there is an unrecoverable error, retries are automatically done
  • Sending order is respected. Media and text messages have the same priority
  • Multiple upload can happen at the same time
  • Tech one: the media being upload is directly stored in the cache with its thumbnail. We do not want to reload it from the matrix media repository once the upload is done and the matrix event is returned by the sync.

Leads

Size estimate

None

Dependencies

  • None

Out of scope

Open questions

Questions

Preview Give feedback
No tasks being tracked yet.

Subtasks

Android

Preview Give feedback
No tasks being tracked yet.

iOS

Preview Give feedback
No tasks being tracked yet.

Other

Preview Give feedback
No tasks being tracked yet.

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

manuroe commented Oct 2, 2024

Notes from a tech discussion in the SDK chapter:

  • From a tech point of view, MSC2246: Asynchronous media uploads was useful to handle the local echo in the timeline but it was an old assumption. With the sending queue we introduced this year, it is no more mandatory to rely on this MSC for the pure purpose of making the media local echo implementation easier in the SDK. It is actually the opposite see below
  • MSC2246 raises new edge cases to manage (there might be more we will discover if weimplement it):
    • Receiver side: the app must repeatly retries to load the content until it is fully uploaded
    • Receiver side: the app needs to compatible with the behaviou4cabove. It means EW, EA, EI and all other Matrix clients.
    • Sender side: the flow involved by the MSC breaks the generic simplicity of the sending queue. The sending queue sends data in a linear order. The MSC2246 with media uploaded asynchronously breaks this flow, leading to a more complex implementation (more bugs, harder to maintain, etc)
    • We need to check if it is compatible with the blur hash
  • We are not confortable to fully use this MSC now as it will create more problems than what it will solve
  • But we raised the idea to use it in a "synchronous way", ie use all the new API to upload the media but send the final matrix event in the timeline only when uploads are done. We are not sure yet about the benefits. It is more an implementation detail we will see when implementing the feature

@mxandreas
Copy link

Not having local echo for media also blocks us currently from showing reasons for sending failure in case the verified identity of the user has changed or a verified user has unverified devices, see details in #2574. Therefore including it this story also into the scope (it is already working for the text messages, now need to make work for file uploads).

@manuroe manuroe changed the title [Story] Local echo for media upload [Story] Upload Local Echo Oct 17, 2024
@manuroe manuroe changed the title [Story] Upload Local Echo [Story] Local echo for media upload Oct 17, 2024
@manuroe
Copy link
Member Author

manuroe commented Oct 22, 2024

To focus with this issue on the core feature, I removed those items:

  • User can edit the caption while uploading
  • The user can cancel the upload
  • In case the sending of the media fails due to a verified user having an unverified device, or identity of the verified-user has changed, the user will see the same error as for text message and can also take similar actions as for a text message (e.g. send anyway and/or withdraw verification)
  • Do we want an upload progress? Or can we reuse the indicator for text message, ie empty circle, then circle with a check?

They will considered in other user stories if we decide to work on them. They are now part of the epic: https://github.com/element-hq/element-internal/issues/433.

@bnjbvr bnjbvr removed their assignment Nov 21, 2024
@bnjbvr
Copy link
Member

bnjbvr commented Nov 21, 2024

All the things in the initial issue have been implemented. Only the subscription to upload progress is missing at the moment, and that has been decided to be OK, considering we have other more urgent priorities.

cc @manuroe, in case you want to close this issue.

@manuroe
Copy link
Member Author

manuroe commented Nov 21, 2024

We want to test and dogfood it in the nighlties until the next RC (Dec, 3rd). We will close the issue at this time.
So far, the feedbacks are great. Great job @bnjbvr !

@manuroe
Copy link
Member Author

manuroe commented Dec 12, 2024

This feature is in the stores since Monday. We get no bug reports, which means the feature just works™️. Let's close this epic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants