Skip to content
This repository has been archived by the owner on Oct 25, 2023. It is now read-only.

add bare-bones storage type toggle #404

Merged
merged 10 commits into from
Jan 21, 2023
Merged

add bare-bones storage type toggle #404

merged 10 commits into from
Jan 21, 2023

Conversation

SergNikitin
Copy link
Collaborator

@SergNikitin SergNikitin commented Jan 18, 2023

This PR adds a very bare-bones way to switch between the type of storage Mazed uses. I want to get something out the door that will allow us to start testing local hosting internally, so I'm looking for the minimal amount of work to make it happen. Looking for feedback what's "good enough" and acceptable!

With that in mind

  • I think the location of the button is probably bad (but it was quickest to add it there) location was moved to a dedicated settings page
  • I think the style of this specific toggle looks visually out of place with the current UI
  • ⚠️ browser has to be restarted after you toggle the setting (or truthsayer has to be refreshed + extension reloaded)
  • ⚠️ to not introduce too much ways to desynchronise the type of storage used between background, content and popup the StorageApi type for content and popup has been changed to unconditionally use a message proxy implementation, so even when the "datacenter" hosting is active, content and popup will no longer send requests to smuggler, they will send a request to background which will pass it to smuggler

TODO:

  • hook up settings value into archaeologist (apply this patch)
  • test how things work when archaeologist is not installed at all
  • polish the visuals at least a little bit
Media1.mp4

@@ -838,63 +763,181 @@ async function initMazedPartsOfTab(
}
}

browser.tabs.onRemoved.addListener(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shows up as a big change, but I just moved most of the background init into an initBackground() function. I did it because before I can call makeStorageApi I need to fetch the AppSettings which is an async call. I couldn't do an async call while keeping the init code structurally the same at the top level of the file since we can't use async there

@SergNikitin
Copy link
Collaborator Author

Toggle was moved to a dedicated page
https://user-images.githubusercontent.com/3685502/213650730-3f92697c-0b57-4f83-9c8e-b6d8c26da9ea.mp4

@SergNikitin SergNikitin marked this pull request as ready for review January 20, 2023 08:31
@akindyakov
Copy link
Contributor

Looks marvellous! Do you mind covering toggle with Posthog feature flag (showing it only for 3 of us)? It's super easy to add : https://eu.posthog.com/feature_flags/new

@akindyakov
Copy link
Contributor

(testing the rest locally, will get back in ~10 min)

@akindyakov
Copy link
Contributor

akindyakov commented Jan 20, 2023

In Chrome. Settings page doesn't work for me, in error log it prints:

log.ts:12 [Mazed/error] Failed to get archaeologist state after 5 attempts. Last error: "Could not establish connection. Receiving end does not exist."
error @ log.ts:12
(anonymous) @ AppSettings.tsx:51
Promise.catch (async)
(anonymous) @ AppSettings.tsx:47
commitHookEffectListMount @ react-dom.development.js:19731
commitPassiveHookEffects @ react-dom.development.js:19769
callCallback @ react-dom.development.js:188
invokeGuardedCallbackDev @ react-dom.development.js:237
invokeGuardedCallback @ react-dom.development.js:292
flushPassiveEffectsImpl @ react-dom.development.js:22853
unstable_runWithPriority @ scheduler.development.js:653
runWithPriority$1 @ react-dom.development.js:11039
flushPassiveEffects @ react-dom.development.js:22820
performSyncWorkOnRoot @ react-dom.development.js:21737
(anonymous) @ react-dom.development.js:11089
unstable_runWithPriority @ scheduler.development.js:653
runWithPriority$1 @ react-dom.development.js:11039
flushSyncCallbackQueueImpl @ react-dom.development.js:11084
flushSyncCallbackQueue @ react-dom.development.js:11072
discreteUpdates$1 @ react-dom.development.js:21893
discreteUpdates @ react-dom.development.js:806
dispatchDiscreteEvent @ react-dom.development.js:4168
content.js:155 [Mazed/debug] [productanalytics] 'archaeologist/content' Attempted to init more than once, returning previously cached instance

Screenshot 2023-01-20 at 08 57 49

@SergNikitin
Copy link
Collaborator Author

Swap this with the ID that got generated for your local version of archaeologist

@akindyakov
Copy link
Contributor

It works with changed ID 🚀 thansk!

@akindyakov
Copy link
Contributor

Connection of quotes to bookmarks is broken in local mode. The scenario is - open a page, safe a quote, save a page. After that all quotes should be right-hand connected to the bookmark. You can check it on a pop up (see screenshot) or in Mazed web app.

Screenshot 2023-01-20 at 09 40 48

@akindyakov
Copy link
Contributor

akindyakov commented Jan 20, 2023

Something is broken in Triptych, right hand side cards are not rendered (even manual ones), see demo:

Screen.Recording.2023-01-20.at.09.44.26.mov

Update 1:
When you adde left hand side note, it also works in a odd way.

Update 2:
3 menu items out of 4 in drop downs (connect to the right and connect to the left) don't work properly. Except "find and connect" - it still works like a charm :)

@akindyakov
Copy link
Contributor

Unexpectedly change behaviour - sorting of notes in search results is reversed. Latest notes have to be on top of the grid and scrolling down you go back in time ⏳

@akindyakov
Copy link
Contributor

Uploading of images doesn't work in local mode, shoes up an error :
Screenshot 2023-01-20 at 09 52 19

Copy link
Contributor

@akindyakov akindyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@SergNikitin SergNikitin merged commit 2d3e0b7 into main Jan 21, 2023
@SergNikitin SergNikitin deleted the storage-api-4 branch January 21, 2023 17:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants