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

Sync trough Github Gists #115

Open
pviscone opened this issue Aug 1, 2022 · 28 comments
Open

Sync trough Github Gists #115

pviscone opened this issue Aug 1, 2022 · 28 comments
Labels
beta 🏗️ Beta feature ; issue stays open for discussions enhancement 🖌 New feature or request

Comments

@pviscone
Copy link

pviscone commented Aug 1, 2022

It could be useful to add the possibility of synchronizing data between different devices.

This can be implemented using GitHub gists (the same method implemented by the old "Settings Sync" extension of VSCode https://marketplace.visualstudio.com/items?itemName=Shan.code-settings-sync )

The things to do are:

  1. Add the possibility to log into Github and to add a token to create ad edit gists
  2. Export the JSON in the gist every time a paper is added or deleted (or manually)
  3. Import the JSON from the gist at the browser startup (or manually)

I really don't know how difficult could be to implement this feature but I think it will be very useful

@pviscone
Copy link
Author

pviscone commented Aug 1, 2022

I was trying to force the browser to sync the extension data with all other settings but I can't locate the data saved by the extension.

This would be a simpler solution, what is the path of the saved data of the extension?

@vict0rsch
Copy link
Owner

vict0rsch commented Aug 2, 2022

Yeah I've thought about this. Other options are: browser.storage.sync instead of browser.storage.local but it's limited to 100kb, or using https://jsonbase.com. I'll look into those options in Sept.

Thanks for your feedback!

@vict0rsch
Copy link
Owner

vict0rsch commented Aug 23, 2022

Hey @pviscone @yongchanghao @julien-blanchon I've been working on this over the past few days. I have an alpha-stage version in #117.

Could you try it out (branch gist-storage) and give me some feedback?

I'm waiting for some opinions before I move any further on the UX/features.

There may be bugs though, make sure you backup your data first (the code should do that for you anyway but better safe than sorry)

@julien-blanchon
Copy link
Collaborator

julien-blanchon commented Aug 23, 2022

Look like it work well, I'm trying out syncing with another computer and I get back to you

Screenshot 2022-08-23 at 17 58 15

@julien-blanchon
Copy link
Collaborator

By the way it seems that "Manifest version 2 is deprecated" : https://developer.chrome.com/blog/mv2-transition/

Screenshot 2022-08-23 at 18 02 44

@vict0rsch
Copy link
Owner

Yeah I'm aware it's gonna be a tough move and I'm postponing... 😅

@julien-blanchon
Copy link
Collaborator

julien-blanchon commented Aug 23, 2022

As a first try, it work pretty well for me.

  • The documentation is fine, maybe hard to find in the option panel.
  • Collision message and auto backup json if hard merge could prevent many misunderstanding.
  • Sync seems fast (≤2sec)

Try on Brave 1.42 on MacOS (Apple Silicon) and Ubuntu.

I'm just wondering if the synchronization procedure with the Github token will be automatic (via the browser sync) or manual. ? A manuel sync is less intuitive, but still easy to setup.

Thanks you very much for this new features, it will be extensively usefull 🥳

@vict0rsch
Copy link
Owner

vict0rsch commented Aug 23, 2022

The documentation is fine, maybe hard to find in the option panel.

Yeah that's kind of expected, it's not meant to be a main feature just yet but there will be a mention on the Readme and in the pop-up menu which will make it more accessible

@vict0rsch
Copy link
Owner

vict0rsch commented Aug 23, 2022

Collision message and auto backup json if hard merge could prevent many misunderstanding.

Collision messages are already implemented are they not? Did you not get an alert asking for a choice?

Also the json backup should have been triggered and it should show up in your downloads. Can you confirm?

@vict0rsch
Copy link
Owner

Sync seems fast (≤2sec)

Cool, my main concern is that it slows down the time-to-display in the pop-up. Keep that in mind and tell me if you end up finding it annoying

@vict0rsch
Copy link
Owner

I'm just wondering if the synchronization procedure with the Github token will be automatic (via the browser sync) or manual. ? A manuel sync is less intuitive, but still easy to setup.

I'm not sure what you mean. What I mean to implement is that once you have entered a valid PAT and clicked on "Start [...]" then you don't ever need to do anything else.

Push and pull are described in the docs but maybe it's not enough?

Of course you need to do the above ^ on all devices you wish to sync

@julien-blanchon
Copy link
Collaborator

Collision message and auto backup json if hard merge could prevent many misunderstanding.

Collision messages are already implemented are they not? Did you not get an alert asking for a choice?

Also the json backup should have been triggered and it should show up in your downloads. Can you confirm?

Yes I did get a the message + prompt to solve the merge (something like erase local, keep local or drop).
After a erase local, the previous local json get download.

@vict0rsch
Copy link
Owner

vict0rsch commented Aug 23, 2022

Re:2

And so that's good right? What would you suggest I change?

(I will implement a merge too, instead of a hard binary choice local/remote)

@vict0rsch
Copy link
Owner

(and thanks for your help!!)

@julien-blanchon
Copy link
Collaborator

I'm just wondering if the synchronization procedure with the Github token will be automatic (via the browser sync) or manual. ? A manuel sync is less intuitive, but still easy to setup.

I'm not sure what you mean. What I mean to implement is that once you have entered a valid PAT and clicked on "Start [...]" then you don't ever need to do anything else.

Push and pull are described in the docs but maybe it's not enough?

Of course you need to do the above ^ on all devices you wish to sync

I mean "chromium" sync the "gist" sync. I have a sync of all my extension + data from extension using Chrome API (for me it's a Brave settings that will encrypt and share the to sync chain, but for most people this sync is done using Google Account). So if I just get to install the extension for the first time on computer 1 and setting up the gist sync, I will also get the extension on computer 2 thought the "chromium" sync. My question was, did I will have to setup again the "gist" sync on computer 2, or this will be sync using extension data. I can't test this at the current state as the app is not syncable because imported from disk. I suppose PaperMemory settings are localstore and something may be needed to make them syncable.

It's not extensively annoying to manually setup the "gist" sync on new computer. It it help not expose the Github Token too much.

@julien-blanchon
Copy link
Collaborator

Re:2

And so that's good right? What would you suggest I change?

(I will implement a merge too, instead of a hard binary choice local/remote)

Yes sound perfect. Nothing to change. Merge features will be welcome but not urgent

@julien-blanchon
Copy link
Collaborator

Sync seems fast (≤2sec)

Cool, my main concern is that it slows down the time-to-display in the pop-up. Keep that in mind and tell me if you end up finding it annoying

Yes, the time-to-display is highly slows downs because of the sync :(. Without internet/sync on it's instant, with it's like between 1/2 sec and 1 sec. It can be annoying now that I've noticed it 😆. Maybe await data and load as usual (and when data get load update the list). Or even just make the fetch not always automatic and add a sync button

@vict0rsch
Copy link
Owner

Oh I see. You're referring to what I mentioned originally, i.e. using storage.sync instead of storage.local when storing data. I can't use that because it's meant for lightweight parameterization, not data sharing and is therefore capped at 100kb. That's too small for us. So yeah you have to setup the token on all devices. But I don't think it's too bad, it's just a one time operation

@vict0rsch
Copy link
Owner

vict0rsch commented Aug 23, 2022

Regarding time to display, I'm indeed thinking of pulling asynchronously and updating the list. The issue with this is that it'll probably be difficult not to disturb your workflow, especially if you're consulting a paper that has been deleted in the pulled data for some reason. It's unlikely but I still have to deal with it. I'll keep it in mind. But I do agree the 0.5-1.5s extra loading time is annoying

@vict0rsch vict0rsch added enhancement 🖌 New feature or request work in progress ⚒ Working on it! labels Aug 23, 2022
@vict0rsch
Copy link
Owner

I've pushed changes:

  • better Options page table of contents to discover features (floating, left)
  • syncing state UI next to the Memory icon (Popup, bottom left)
  • asynchronous pull from remote to come back to the fast time-to-display

@julien-blanchon can you pull the updated code from gist-storage and tell me how it works for you?

In particular the difficult scenario is

  • Memory changes on Device 1
  • User opens Popup on Device 2
  • Fetching of papers starts but user opens the Memory before it's done (on Device 2)

What I expect is: things will change the next time the user interacts with the Memory, i.e. new search new tag filter or simply memory closing and re-opening. But they should NOT interfere with your current interaction (for instance, it should not reset the search if you've already typed characters in the search bar and the pre-sync memory was filtered displayed accordingly)

@julien-blanchon
Copy link
Collaborator

julien-blanchon commented Aug 24, 2022

Great I'll try syncing when I get home.

New version is install on Device 1 (macbook) currently. Main tab open instantly and sync in background at each open (Syncing ... and then Sync ✅). Memory Tab also open instantly even if opened before the end of Sync.

The new Option page is pretty neat, nice animation (I didn't know that X engineer were ALSO so good in UX 😵). The #header padding does not render is some case, especially when reducing the size of the window. Maybe disable or reducing the padding.

Otherwise the rest seems to continue to work normally

@vict0rsch
Copy link
Owner

Great! I'll adjust the UI. Thanks!

@julien-blanchon
Copy link
Collaborator

julien-blanchon commented Aug 24, 2022

I thought that everything would work as it should at first glance but it seems that there is some bug.
When I remove article then it get remove from the local app but aren't really update in the gist.

(revised but with no changes)
Screenshot 2022-08-24 at 20 33 51

EDIT: + In fact the gist is revised (with nothing) at each sync. And delete did not trigger a remove in the gist.

I don't know if it's new (I hadn't tried remove the first time).
I will keep using this version as a beta tester until it get release, let me know if you spot other bugs

@vict0rsch
Copy link
Owner

I have noticed issues with deletions too. Not sure where it comes from but I'll investigate

@vict0rsch
Copy link
Owner

@julien-blanchon I've pushed changes to the branch, can you pull and check things work as expected?

@julien-blanchon
Copy link
Collaborator

julien-blanchon commented Aug 31, 2022

I did some test on b8cbe34 and figure the following things :

  • Each time I open the extension, it sync and write a revised in my gist (without change but still it could inflate traking)

Screenshot 2022-08-31 at 19 13 50

  • Some strange behaviour when removing/addings paper while extension is open on another device. But it's not a real use case. Like I delete a paper on device 1, then close the extension on device 2 (with the paper) so it will push back the deleted paper (again it's fine like that).

  • I could trick the app if when I go to fast when adding a new paper (cf following screenshot). But it's a edge case that really happen in real workflow: Screenshot 2022-08-31 at 19 21 46

In term of new features i can confirm that the following things work :

  • Pull, Push and Ok status preview (in the extension icon & option page)
  • Removing paper work (something like 1sec)
  • Fast extension load time (open instantly as the main branch does)

@vict0rsch
Copy link
Owner

Yes, I've updated the 1st time user experience with a warning that says that the current implementation is intended for workflows that allow at least a 5-10s gap between devices. I don't think I can make it faster using gists. Especially when I'll move to the manifest V3 which removes permanent background pages and therefore will slow things down by a second or two

@vict0rsch
Copy link
Owner

I'm releasing in alpha mode with an important constraint I don't have the time to address right now: because of Github's cache, data won't be updated for 30-60 seconds.

Help is wanted to move this feature to beta then stable release.

@vict0rsch vict0rsch added beta 🏗️ Beta feature ; issue stays open for discussions and removed work in progress ⚒ Working on it! low priority 🐢 labels Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta 🏗️ Beta feature ; issue stays open for discussions enhancement 🖌 New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants