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

Implement basic push notification logic #6244

Draft
wants to merge 8 commits into
base: master
from

Conversation

@tuarrep
Copy link
Contributor

tuarrep commented Dec 28, 2019

Purpose

Implement native browser push notifications on some important events. Closes #5329.

Status

For now notifications are handled client-side, the tab must be always open (but not necessarily visible) to receive notifications.

Events

As discussed here some events need to be notified to user.
Here the status of implementation

Event Status Comments
GUI looses connection to Syncthing Also notify when connection is restored
A error has occurred on the GUI-side (e.g. parsing error when reading the JSON returned by Syncthing's API) Same as network connection losing
A notification has been emitted through Syncthing's API (yes, there's already an API for that) x
The synchronization of a local folder has been completed
The synchronization of a remote folder on device X has been completed
A new device wants to connect x
A device wants to share a folder

Global

  • Add option in setting panel
  • Make notifications messages translatable

Screenshots

Note: Notifications are shown by the browser and the operating system. Depending of these the style could be different of mine. If you already saw notifications from a website, it will be shown like these.

screencapture-127-0-0-1-8384-2019-12-28-16_16_34
image
image

Testing

First, you need to enable notifications in settings panel as show in the screenshot above.
I your browser doesn't support push notification or you've blocked them, the checkbox is disabled.

A notification is shown when :

  • The GUI lost connection with the server: turn off the server
  • The GUI restore the connection with the server : turn back on the server
  • A local folder finish syncing: Add a file on another device in the shared folder
  • A device wants to share a folder: Share a folder from another device. A notification is shown as the GUI alert appears
  • The synchronization of a remote folder on device X has been completed : Create a reasonably large file (if it is too small GUI doesn't have time to detect it before it was synced) on your local computed and wait until it was synced to a remote device. You can use dd if=/dev/random bs=1024 count=50000 iflag=fullblock to create a 50MB file with random bytes.

I've drafted this PR to have feedback on implementations and priorities before going too deeper in it.
I'll be happy to here back from you about this feature.

@Catfriend1

This comment has been minimized.

Copy link

Catfriend1 commented Dec 29, 2019

Hi,

This seems pretty nice to me to make the bare Syncthing more natively supporting things the most third party wrappers manually added in the past. We would gain advantage by not having to maintain status checks and notifications cross platform then.

@AudriusButkevicius

This comment has been minimized.

Copy link
Member

AudriusButkevicius commented Jan 23, 2020

Do you have a screenshot of what this looks like?

Copy link
Member

AudriusButkevicius left a comment

Looks like a good thing to build on, yet I suspect we'll have users screaming that they don't want this, or screaming that they want more (up to the levels of silly notification per file). Perhaps this could be configurable.

I've asked a few questions to see how we're taking this forward.

gui/default/syncthing/core/syncthingController.js Outdated Show resolved Hide resolved
@tuarrep tuarrep force-pushed the tuarrep:push-notifications branch from 52e5d41 to 844afb1 Jan 26, 2020
@tuarrep tuarrep force-pushed the tuarrep:push-notifications branch from 9ab1864 to 3e34dd7 Mar 3, 2020
tuarrep added 5 commits Dec 28, 2019
@tuarrep tuarrep force-pushed the tuarrep:push-notifications branch from 4412e3b to 4da78bd Mar 22, 2020
tuarrep added 2 commits Mar 22, 2020
@tuarrep tuarrep force-pushed the tuarrep:push-notifications branch from d974996 to 8f5abe9 Mar 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.