-
-
Notifications
You must be signed in to change notification settings - Fork 432
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
feat: namespaced keychain #428
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/standardnotes/web/blob/004-namespaced-keychain/app/assets/javascripts/ui_models/application.ts#L59
The WebApplication class still defines the namespace as an empty string. Should it be injected instead?
Also how will the application get the keychainValue from an 003 installation?
I guess if namespace
stays an empty string this works with 003 installations but then doesn't achieve namespacing completely.
I suppose it might make sense to think about how multiple application namespaces would look like. We will need to persist Something like: /** Before creating an application */
const namespaces = localStorage.get("namespaces") || [];
const namespace = `application-${namepaces.length}`;
const application = new Application(..., namespace, ...); |
I suppose we can address this when we get to actually run multiple applications at once. I just thought that was the intent of the feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also update the snjs dependency, I'm assuming that's why the type check is failing :)
Related: standardnotes/snjs#13