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

feat: add API to save messages #5606

Merged
merged 23 commits into from
Jan 19, 2025
Merged

feat: add API to save messages #5606

merged 23 commits into from
Jan 19, 2025

Conversation

r10s
Copy link
Member

@r10s r10s commented May 20, 2024

took quite some time until i found the time to finish this PR and to find a time window that does not disturb other developments too much, but here we are:

this PR enables UI to improve "Saved messages" hugely, bringing it on WhatsApp's "Starred Messages" or Telegram's "Saved Messages" level. with this PR, UIs can add the following functionality with few effort (~100 loc on iOS):

  • add a "Save" button in the messages context menu, allowing to save a message
  • show directly in the chat if a message was saved, eg. by a little star ★
  • in "Saved Messages", show the message in its context - so with author, avatar, date and keep incoming/outgoing state
  • in "Saved Messages", a button to go to the original message can be added
  • if the original message was deleted, one can still go to the original chat

these features were often requested, in the forum, but also in many one-to-one discussions, recently at the global gathering.

moreover, in contrast to the old method with "forward to saved", no traffic is wasted - the messages are saved locally, and only a small sync messages is sent around

this is how it looks out on iOS:

Screenshot 2025-01-17 at 00 02 51     Screenshot 2025-01-17 at 00 05 33

technically, still a copy is done from the original message (with already now deduplicated blobs), so that things work nicely with deletion modes; eg. you can save an important message and preserve it from deletion that way.

jsonrpc can be done in a subsequent PR, i was implementing the UI on iOS where that was not needed (and most API were part of message object that is not in use in jsonrpc atm)

@hpk42 the forward issue we discussed earller that day is already solved (first implementation did not had an explict save_msgs() but was using forward_msgs(SELF) as saving - with the disadvantage that forwarding to SELF is not working, eg. if one wants the old behaviour) acutally, that made the PR a lot nicer, as now very few existing code is changed

previous considerations and abandoned things

while working on this PR, there was also the idea to just set a flag “starred” in the message table and not copy anything. however, while tempting, that would result in more complexity, questions and drawbacks in UI:

  • delete-message, delete-chat, auto-deletion, clear-chat would raise questions - what do do with the “Starred”? having a copy in “Saved Messages” does not raise this question
  • newly saved messages appear naturally as “new” in “Saved Messages”, simply setting a flag would show them somewhere in between - unless we do additional effort
  • “Saved Messages” already has its place in the UI - and allows to directly save things there as well - not easily doable with “starring”
  • one would need to re-do many things that already exist in “Saved Messages”, at least in core
  • one idea to solve some of the issues could be to have “Starred” as well as “Saved Messages” - however, that would irritate user, one would remember exactly what was done with a message, and understand the fine differences

whatsapp does this “starred”, btw, so when original is deleted, starred is deleted as well. Telegram does things similar to us, Signal does nothing. Whatsapp has a per-chat view for starred messages - if needed, we could do sth. like that as well, however, let’s first see how far the “all view” goes (in contrast to whatsapp, we have profiles for separation as well)

for the wording: “saving” is what we’re doing here, this is much more on point as “starring” - which is more the idea of a “bookmark”, and i think, whatsapp uses this wording to not raise false expectations (still, i know of ppl that were quite upset that a “starred” message was deleted when eg. the chat was cleared to save some memory)

wrt webxdc app updates: options that come into mind were: empty (as now), snapshot (copy all updates) or shortcut (always open original). i am not sure what the best solution is, the easiest was empty, so i went for that, as it is (a) obvious, and what is already done with forwarding and (b) the original is easy to open now (in contrast to forwarding).
so, might totally be that we need or want to tweak things here, but i would leave that outside the first iteration, things are not worsened in that area

wrt reactions: as things are detached, similar to webxdc updates, we do not not to show the original reactions - that way, one can use reactions for private markers (telegram is selling that feature :)

to the icon: a disk or a bookmark might be other options, but the star is nicer and also know from other apps - and anyways a but vague UX wise. so i think, it is fine

finally, known issue is that if a message was saved that does not exist on another device, it does not get there. i think, that issue is a weak one, and can be ignored mostly, most times, user will save messages soon after receiving, and if on some devices auto-deletion is done, it is maybe not even expected to have suddenly another copy there

EDIT: once this is merged, detailed issues about what to do should be filed for android/desktop (however, they do not have urgency to adapt, things will continue working as is)

@r10s r10s force-pushed the r10s/improve-saved-messages branch 8 times, most recently from 6ca3eb3 to 1440885 Compare October 17, 2024 10:41
@r10s r10s force-pushed the r10s/improve-saved-messages branch from 1440885 to eb4a407 Compare October 21, 2024 19:40
@r10s r10s force-pushed the r10s/improve-saved-messages branch 2 times, most recently from c4f89bb to 8585e9c Compare November 11, 2024 17:44
@r10s r10s force-pushed the r10s/improve-saved-messages branch from 8585e9c to cbd5394 Compare November 29, 2024 20:05
@r10s r10s force-pushed the r10s/improve-saved-messages branch 2 times, most recently from 166f387 to 93dc46c Compare January 14, 2025 15:39
@r10s r10s force-pushed the r10s/improve-saved-messages branch from 93dc46c to bf72686 Compare January 16, 2025 14:29
@r10s r10s force-pushed the r10s/improve-saved-messages branch from eb147f0 to 33e1a9c Compare January 16, 2025 15:49
@r10s r10s marked this pull request as ready for review January 16, 2025 23:31
@r10s r10s added the enhancement New feature or request label Jan 17, 2025
Copy link
Collaborator

@Hocuri Hocuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partial review, I just commented some very small suggestions on tests

src/chat.rs Outdated Show resolved Hide resolved
src/chat.rs Outdated Show resolved Hide resolved
src/sync.rs Outdated Show resolved Hide resolved
r10s and others added 3 commits January 17, 2025 17:15
Co-authored-by: Hocuri <hocuri@gmx.de>
Co-authored-by: Hocuri <hocuri@gmx.de>
Co-authored-by: Hocuri <hocuri@gmx.de>
src/message.rs Show resolved Hide resolved
src/chat.rs Show resolved Hide resolved
src/chat.rs Outdated Show resolved Hide resolved
deltachat-ffi/deltachat.h Outdated Show resolved Hide resolved
deltachat-ffi/deltachat.h Outdated Show resolved Hide resolved
deltachat-ffi/deltachat.h Outdated Show resolved Hide resolved
r10s added 4 commits January 19, 2025 00:10
agree with @Hoccuri after thinking it over,
the limited cornercase of deleted message but not deleted chat
seems not to be worth additional effort and states -
and may even worsen UX as it is unclear what happens when tapping the
"goto original" button (or one need to use different icons or sth)

if we want to do sth. about that,
it might be better to go the way we went for quotes:
so, eg. keep a copy of the chatname.
but let's see if this is really needed and an issue at scale.
@r10s
Copy link
Member Author

r10s commented Jan 19, 2025

thanks a lot for the great, detailed review, @Hocuri ❤️ - i tried to target all comments

@r10s r10s requested a review from Hocuri January 19, 2025 13:05
Copy link
Collaborator

@Hocuri Hocuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

deltachat-ffi/deltachat.h Outdated Show resolved Hide resolved
Co-authored-by: Hocuri <hocuri@gmx.de>
@r10s r10s changed the title improve saved messages feat: add API to save messages Jan 19, 2025
@r10s r10s changed the title feat: add API to save messages feat: add API to save messages Jan 19, 2025
@r10s r10s enabled auto-merge (squash) January 19, 2025 15:34
@r10s r10s merged commit 0d8c2ee into main Jan 19, 2025
37 checks passed
@r10s r10s deleted the r10s/improve-saved-messages branch January 19, 2025 15:44
@ralphtheninja
Copy link
Member

Amazing! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants