darktrojan / newtabtools

New Tab Tools add-on for Firefox and Chrome
https://addons.mozilla.org/firefox/addon/new-tab-tools/
255 stars 73 forks source link

storage.sync #336

Open yfdyh000 opened 7 years ago

yfdyh000 commented 7 years ago

https://github.com/darktrojan/newtabtools/search?q=local

Maybe you should allows storage.sync?

mjbogusz commented 7 years ago

While I see this as an important addition as a long-time NTT user, I don't think a simple search&replace will do.

Here are some cases and possible issues that I think have to be addressed:

  1. Whether to force a synchronisation
  2. What to do in case of mismatching configurations

As for 1, I think there should be a toggle that would enable copying settings between storage.local and storage.sync. While some people might want to have same settings on all machines, some would prefer having them different, e.g. to adjust for different screen sizes. This approach (copying) should also have the lowest chance of breaking something, as it would minimise changes to already working code.

Possible future expansion of this functionality would be a list of checkboxes for things to sync, like theme, columns, spacing, filters etc (basically properties or group of properties of Prefs object).

As for 2, I think for now relying on already-stored Prefs.versionLastUpdate settings update time would be enough, but perhaps it would be better to display a notification about that fact with few options, like "overwrite local with synced", "overwrite synced with local", "stop sync".

I think I had one more possible edge case but I can't recall it now...


In the meantime, I think manual import/export by JSON.stringify() and .parse() on the Prefs object should be possible - displaying a text field with arbitrary value should be possible even with WebExt API ;)

mjbogusz commented 7 years ago

Just to add to that manual import/export thing:

Thumbnails would obviously have to be handled separately from the preferences, but I've managed to get raw image Blob data from them with Tiles.getAllTiles() and then converted that to base64 with

let base64TileImage;
let fr = new FileReader();
fr.onload = function (event) {
    base64TileImage = btoa(String.fromCharCode(...new Uint8Array(event.target.result)));
};
fr.readAsArrayBuffer(tile.image); // 'tile' is one object from Tiles.getAllTiles() array

Of course onload is asynchronous and we'd run it in a loop so it would require something like wrapping it in a Promise and then doing a Promise.all().

Blob->base64 magic based on:

That's not enough for a PR yet but I'm considering creating one.

darktrojan commented 7 years ago

Thanks for the good comments on this @mjbogusz.

In the meantime, I think manual import/export by JSON.stringify() and .parse() on the Prefs object should be possible - displaying a text field with arbitrary value should be possible even with WebExt API ;)

That's already stored in a JSON file by the way, go look in your profile under browser-extension-data/newtabtools@darktrojan.net.

Speaking of tile images, you wouldn't be able to put them in storage.sync as it has a 100kB limit. I'd hit that with five images.

I should be able to bring back the import/export functions soon-ish. It's not a priority, but I have some ideas I've been looking into. Hopefully that will be more use to people than sync.

mjbogusz commented 7 years ago

Posting here mostly for cross-issue reference and notification for @yfdyh000:

I've added a comment to the export/import issue (#287) with a simple tiles import/export script. That along with copying storage.json for settings should be enough of a workaround for now.