I'm thinking how to best implement this and I'd like to ask laurent22's advice. I can see two main ways of doing it:
Checking network type each time in app-mobile/root before calling setupRecurrentSync and aslo before starting initial sync on app launch.
Adding an optional callback function to lib/registry (eg canSync) which is called before each scheduled sync. Then this function is defined in app-mobile/root to check the network type. This would require less new code and it's resilient to all future changes in how setupRecurrentSync is called. On the other hand extra logic is necessary to allow for manual syncs. (which, I assume, should still always be usable)
How do you check if the wifi is on or off? I guess you shouldn't poll for this as it's a native call, which would be slow. Instead there should be some event you can listen to, is that correct?
For this you should keep things as low level as possible so that any place that use the sync feature will go through your wifi check, so your second approach I think is best. In registry.ts somewhere there's a call to sync.start() and I think that's just before this function that you should add your checks.
Keep in mind that registry.ts is used on all platforms (including desktop and cli) so make sure your check is active only for mobile.
Also be careful when you make the native call to check the wifi state, make sure you handle errors. If there's an error, it should default to have the wifi enabled as we can't risk having sync disabled due to some error. But the error of course should be logged.
The spec on that issue is not well detailed, so if you could describe what you're going to do before implementing it that would be great. We need to know:
How the UI will look (any new options, UI element, etc.). No need for a mockup, just a description in text is enough.
An overall idea of how you're going to implement it.
It supports both polling and subscribing to changes in network state. I think initially I'll just poll it each time from registry.ts as you suggested and then see if it impacts performance. If it does I'll add an event listener and keep track of the network state in a member field.
As for UI I'd like to keep it minimal, I'd just add a mobile-only toggle option (like "Synchronize only over WiFi connection") to Settings.ts, and also some indication above the sync button if automatic syncing is disabled because of mobile data. "Using mobile data, automatic synchronization is disabled. You can still press Synchronize to start a manual sync." for example.
As I mentioned, manual sync should still be possible so I think for that I'd pass an argument to scheduleSync to ignore network check
Also I've noticed while testing that on the dev branch this report under the sync button doesn't show up after sync, and the animation doesn't play while syncing either. I've traced it back to lib/Synchronizer.ts, where it seems, the dispatch method is not correctly set, but I can't figure out why. I've tested it on abe0013 so it's not caused by my changes. (the last screenshot comes from my phone running 1.7.5)