isamert / scli

a simple terminal user interface for signal messenger (using signal-cli)
GNU General Public License v3.0
432 stars 40 forks source link

Toggle reaction notifications, fix #176 #174 #177

Closed cyrinux closed 2 years ago

cyrinux commented 2 years ago

Hi here,

Please find present from @maximbaz and me to fix #176 and #174 .

Thanks for the project 😊

exquo commented 2 years ago

Thanks guys!

I've simplified the send_desktop_notification() usage. If we choose to not show notifications in the status line, all the changes can be reduced to a single line:

    def _on_receive_reaction(self, envelope):
         msg = self._add_reaction(envelope)
-        if is_envelope_outgoing(envelope):
+        if is_envelope_outgoing(envelope) or not cfg.notify_on_reactions:
         return

On the one hand, not using --notify-on-reactions seems to imply no notifications of any kind - either desktop or in the status line. On the other hand, we currently apply all notification.. options just to desktop notifications..

I've made some other minor changes - rewording, etc. Also, removed the line from readme, since that is just an example.

Let me know what you think!

maximbaz commented 2 years ago

Nice, thanks for the cleanup! Looks good to me.

I think it does make sense to show notification in the status bar, it's not distracting, not confusing, and at the same time could be informative.

cyrinux commented 2 years ago

Cool, it make also sense for me, thanks @exquo .

exquo commented 2 years ago

Sounds good. Let's keep the status line notifications. Thanks!