Skip to content

notify adding reactions#6072

Merged
r10s merged 20 commits intomainfrom
r10s/notify-reactions
Oct 21, 2024
Merged

notify adding reactions#6072
r10s merged 20 commits intomainfrom
r10s/notify-reactions

Conversation

@r10s
Copy link
Copy Markdown
Contributor

@r10s r10s commented Oct 19, 2024

this PR adds an event for reactions received for one's own messages.

this will allow UIs to add notification for these reactions.

Screenshots at deltachat/deltachat-ios#2331 and deltachat/deltachat-android#3377

@link2xt
Copy link
Copy Markdown
Collaborator

link2xt commented Oct 20, 2024

I thought Android relies on getFreshMsgs to return what should currently be in the notifications, but apparently it is not the case.

Wonder if we should deprecate getFreshMsgs and replace it with getFreshMsgCount(0) or something like this. Counting messages should be cheaper than passing arrays around.

@r10s
Copy link
Copy Markdown
Contributor Author

r10s commented Oct 20, 2024

at least on android, notifications are driven by DC_EVENT_INCOMING_MSG - and with this PR also DC_EVENT_INCOMING_REACTION.
i did not look further into getFreshMsg() when i realised that the reactions have no ID :)

but yes, it seems at least on android only getFreshMsg().length is used - having that as getFreshMsgsCount(0) would be nicely fitting to other places where the API uses this pattern. i did not check other OS yet, however

@r10s r10s marked this pull request as ready for review October 20, 2024 16:35
@r10s r10s changed the title notify reactions notify adding reactions Oct 20, 2024
@r10s r10s force-pushed the r10s/notify-reactions branch 2 times, most recently from a9d3db6 to 3661482 Compare October 21, 2024 11:03
@r10s r10s force-pushed the r10s/notify-reactions branch from 3661482 to 28c60c3 Compare October 21, 2024 11:05
@r10s r10s force-pushed the r10s/notify-reactions branch from 21be89d to 1cabfb6 Compare October 21, 2024 11:59
@r10s r10s requested a review from iequidoo October 21, 2024 14:24
@r10s r10s requested a review from iequidoo October 21, 2024 19:05
@r10s
Copy link
Copy Markdown
Contributor Author

r10s commented Oct 21, 2024

thanks a lot, @iequidoo, for review and the many tips! ❤️

@r10s r10s merged commit 61fd0d4 into main Oct 21, 2024
@r10s r10s deleted the r10s/notify-reactions branch October 21, 2024 19:35
WofWca added a commit that referenced this pull request Feb 1, 2025
@WofWca WofWca mentioned this pull request Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants