search

toolbox-team / reddit-moderator-toolbox

Moderator toolbox for reddit extension development.
https://www.reddit.com/r/toolbox
Apache License 2.0
114 stars 38 forks source link

Brainstorm: Use `browser.storage.sync` for syncing Toolbox settings #226

Open eritbh opened 4 years ago

eritbh commented 4 years ago

This API gives us access to storage that's shared in more or less real time across all browsers that a user is logged in on. I'm not entirely sure how it works for users that aren't signed in, and it may turn out that this would require us to check both storage.sync and storage.local on all our sets and lookups, but it'd be a nice convenience for people that work on multiple machines under a consistent browser if it's easy to implement.

creesch commented 4 years ago

Hrm... there used to be some serious limits in the amount of data you could store in the chrome variant of storage sync.

Also, speaking personally I do have different toolbox settings based on where I am. At work I have notifications turned off entirely as I only mod when I have a bit of time. On android I have a bunch of stuff turned off.

So I am not entirely against it but am also not convinced toolbox needs this.

eritbh commented 4 years ago

Hrm... there used to be some serious limits in the amount of data you could store in the chrome variant of storage sync.

It's limited to 100KB of JSON data, which we can definitely work with.

Also, speaking personally I do have different toolbox settings based on where I am. At work I have notifications turned off entirely as I only mod when I have a bit of time. On android I have a bunch of stuff turned off.

I don't think this should be on by default, it would basically be an alternative to the existing "backup to a subreddit wiki" feature, but a bit less jank than having to have a privated personal subreddit to save your settings in.

creesch commented 4 years ago

As long as it is an option. In that regard we do have a few ways we might want to handle this so people can work with it in a flexible manner:

The most sensible way to implement this is when people enable this to check if there are synced settings available. With two possible outcomes.

  1. No settings available, sync the current local settings.
  2. Synced settings available, overwrite current settings.

Meaning that we need to place a nice warning so people know what to expect. I'd also like to provide an option to force sync settings of the current toolbox installation so that people have an option when they enable it first on a different installation but realize those are not the settings they want to fix it.

And while writing this I got a really strong sense of deja vu because I realized I already did this one for the lounge irc client. 😁

This is what I mean basically:

image

eritbh commented 4 years ago

I think it might be better to just give the user a choice of what to do rather than trying to infer the correct behavior. If there's data in storage.sync then the user should choose which settings to keep when enabling the sync feature - either they overwrite the sync data with local settings, or vice versa. If there's nothing already in the sync storage then just automatically push local settings there.

Alternatively, I kinda like the idea of just shipping new installations (not existing installations) with syncing enabled, so then the initial settings are automatically pulled from the cloud and if the user doesn't want to use syncing on that installation then they can just disable it and then modify settings. This might have issues though, if people don't know the settings sync is a thing and want to maintain separate configs for different devices without disabling the sync first.

creesch commented 4 years ago

What I proposed does give them that choice though. Of course it needs to fit the toolbox settings dialogue.

The point is that we need to do it in such a way that users know what to expect, don't "loose" settings and still make it easy to use.

You can't make this a per setting thing, for starters we simply don't store settings separately but as one json object (which is good looking at the storage issues RES has had with firefox storage) but also because that makes things overly complex for most users. We already have a ton of settings as it is, you don't want to add an extra setting for each setting to have people decide if they maybe want to sync that setting or not. Specifically because a lot of settings are also related and might cause unexpected behavior for users if one is synced and the other is not.

So you either sync all your settings or you don't. The only question then remains is what settings are used as "master" which is why I proposed what I did as it is a fairly easy flow.

This might have issues though, if people don't know the settings sync is a thing and want to maintain separate configs for different devices without disabling the sync first.

Exactly, for the irc client we had a whole lot of discussion about all the possible ways to approach this and how it could go wrong.