search

syncthing / syncthing-android

Wrapper of syncthing for Android.
https://syncthing.net/
Mozilla Public License 2.0
3.16k stars 362 forks source link

Provide notification handler in device and folder activities #2055

Closed adamszewe closed 3 months ago

adamszewe commented 5 months ago

wip, waiting for https://github.com/syncthing/syncthing-android/pull/2054 to merge first

Decouples a bit FolderActivity and DeviceActivity from SyncthingService by injecting NotificationHandler into them directly instead of relying on a method in SyncthingService to provide it for them.

imsodin commented 4 months ago

How does this relate to #2054? Doesn't look like preferences are involved here.

With the previous way, only a single notification handler instance. Is it ok to have multiple?

adamszewe commented 4 months ago

How does this relate to #2054? Doesn't look like preferences are involved here.

With the previous way, only a single notification handler instance. Is it ok to have multiple?

This PR is just waiting for the first one to get merged so I can rebase this one. I left it in draft as it's still pointing to the main branch even though it has changes on it from the first PR.

In this PR I want to refactor how the NotificationHandler is provided the FolderActivity and DeviceActivity. Currently these classes are getting a reference to the NotificationHandler from SyncthingActivity, which couples them for no good reason. We can just inject NotificationHandler into those classes. NotificationHandler is already being provided as a s Singleton by Dagger, so there is going to be only a single instance of it in the app.

acolomb commented 3 months ago

Are you closing this just for lack of review / feedback? Or is there some different plan?