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

improve saved messages #3073

Merged
merged 13 commits into from
Feb 7, 2025
Merged

improve saved messages #3073

merged 13 commits into from
Feb 7, 2025

Conversation

r10s
Copy link
Member

@r10s r10s commented May 20, 2024

close #3573

to make place for the "Save" icon, the "Info" icon, which is anyway more a debug view currently, is moved to the overflow menu.

currently, only one selected message can be saved per action, mutli-select-mode can be done if needed in a subsequent PR

what is missing and should go to a subsequent PR, is a button to go from the "saved message" to the "original message"; my android-layout-constraints skills are too unused to target that in reasonable time :)

@r10s r10s force-pushed the r10s/improve-saved-messages branch from 81abf44 to daa9ffe Compare May 20, 2024 16:44
@r10s r10s force-pushed the r10s/improve-saved-messages branch from daa9ffe to 5f16db0 Compare June 12, 2024 14:31
@r10s r10s force-pushed the r10s/improve-saved-messages branch 4 times, most recently from c96ca16 to dd8cd64 Compare October 16, 2024 15:34
@r10s r10s force-pushed the r10s/improve-saved-messages branch from af1ba4a to d625033 Compare January 13, 2025 16:20
@r10s
Copy link
Member Author

r10s commented Jan 16, 2025

closing, api has changed, this can serve as a template, acutal prototyping was done on iOS at deltachat/deltachat-ios#2527

@r10s r10s closed this Jan 16, 2025
@r10s r10s mentioned this pull request Feb 1, 2025
5 tasks
@r10s r10s reopened this Feb 2, 2025
@r10s r10s force-pushed the r10s/improve-saved-messages branch 2 times, most recently from 66ab287 to a7fa7e3 Compare February 2, 2025 11:31
Copy link

github-actions bot commented Feb 2, 2025

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@r10s r10s force-pushed the r10s/improve-saved-messages branch from a7fa7e3 to ea2b104 Compare February 2, 2025 12:17
Copy link

github-actions bot commented Feb 2, 2025

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@r10s r10s force-pushed the r10s/improve-saved-messages branch from ea2b104 to 7d28292 Compare February 2, 2025 12:47
@r10s r10s requested a review from adbenitez February 2, 2025 12:58
@r10s r10s marked this pull request as ready for review February 2, 2025 12:59
Copy link

github-actions bot commented Feb 2, 2025

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@r10s
Copy link
Member Author

r10s commented Feb 2, 2025

/me adapted to new api, updated pr description

@adbenitez adbenitez force-pushed the r10s/improve-saved-messages branch from 7d28292 to d1f77d9 Compare February 5, 2025 15:34
@adbenitez
Copy link
Member

I am testing the PR a bit, some comments at a first glance:

  • appending the star as text to the footer label is not good, localization-wise but also it looks weird for outgoing messages, it should probably be at the beginning for outgoing messages, not mixed between data label and delivery checkmark
  • normally sent messages in "saved messages" chat don't have the star, that is ok, but then when selecting the message the star button is not hidden for those, instead it is available and if you click it you get an option to remove the message which is basically the same duplicated action of the trash icon besides, I think it is better to just hide the option for messages directly sent in saved messages
  • the previous point also applies to messages that its "parent"/original message was deleted, I think it is better to just show the delete trash can there and not show the "un save" button, because it is a bit confusing, if saving/unsaving sometimes show the same dialog than the trash can I can already see the problems of people accidentally deleting messages with the trash can when wanting to un-star etc
  • I find the "save/unsave" terms a bit confusing wrt meaning, saving sounds more like the "export attachments" option, probably "bookmark" or "star" would have fit better, but maybe it is too late for renamings

Copy link

github-actions bot commented Feb 5, 2025

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@adbenitez
Copy link
Member

adbenitez commented Feb 5, 2025

For other testers: if you are confused why messages directly sent in saved messages doesn't have a padlock, it was a bug in core, it was fixed and it will show a padlock when we upgrade to core 1.155.2 or superior

@r10s
Copy link
Member Author

r10s commented Feb 5, 2025

  • star/bookmark naming was considered (among tons of other things, see previous discussions) but there the expectation is more, that star/bookmark gets deleted when original is deleted. also some ppl will confuse that with sort of reaction. what we're doing is actually saving. still, the icon is a star, but that ia not that unusual. but, yes, that discussion is too late :)
    let's move forward

  • i agree that a dedicated view is better than a concatenated string, feel free to do a subsequent pr, that viewstuff is above my paygrade :)
    still, the star should appear beside the date, but if think, this is not questioned

  • for hiding "unsave" for messages added directly to "saved messages": on iOS, we had it like that before, but it was confusing. but the menu is different on iOS (delete a tap more far away) - so, i am fine with doing it differently on android. consider, however, ppl do not really remember how the one or the other message was saved, and do not understand why "unsave" is sometimes there and sometimes not. we can also consider to remove "unsave" completely in "saved messages" and let ppl just hit "delete" - that would skip that uncertainity.
    but i would leave that change also for a later PR

@adbenitez adbenitez added the enhancement actually in development, user visible enhancement label Feb 7, 2025
Copy link

github-actions bot commented Feb 7, 2025

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@adbenitez adbenitez enabled auto-merge February 7, 2025 17:16
@adbenitez adbenitez merged commit 257e200 into main Feb 7, 2025
2 checks passed
@adbenitez adbenitez deleted the r10s/improve-saved-messages branch February 7, 2025 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement actually in development, user visible enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

adapt to new saved-messages api
2 participants