This repository was archived by the owner on Oct 22, 2024. It is now read-only.
Merged
Conversation
1aff5c1 to
208c8df
Compare
34d2cdc to
cc08a0c
Compare
cryptotavares
commented
Feb 24, 2023
| "vinyl-sourcemaps-apply": "^0.2.1", | ||
| "wait-on": "^7.0.1", | ||
| "watchify": "^4.0.0", | ||
| "webextension-polyfill": "^0.8.0", |
Contributor
Author
There was a problem hiding this comment.
it was duplicated as a dev and prod dep... removed the dev dep
matthewwalsh0
approved these changes
Mar 3, 2023
vinistevam
approved these changes
Mar 4, 2023
cc08a0c to
ce181fb
Compare
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
3808a1a to
1f855e9
Compare
Show and hide window using background controller listeners.
electron BrowserWindows do not have the property chrome.runtime. In fact, unless we are loading an extension, it will not have any of the browser extension API properties. In order to polyfill those we are relying on the webextension-polyfill package. But unlike the background process, we do not have chrome.runtime within electron Browser window (as there is no extension id, because we are not loading an extension). We are patching webextension-polyfill to bypass checking for chrome.runtime and chrome.runtime.id
Notification manager expects a window object with left prop to be returned when creating a popup window.
Replicate the behaviour from home component on the extension. Confirmation and permission pages route to the '/' (home) page. And then the home page decides if the notification popup window should be closed or not.
There are a couple of actions that call methods from global.platform. Those are not available in the electron BrowserWindow and we do not need them. To prevent unhandled promise rejections we are overriding those to noop
Split styles bundle for both desktop ui and popup ui.
This will allow us to shim the browser api within the desktop popup window without requiring any changes to the Extension repo
if desktop popup is not enabled the notification-manager is still expecting a window create/update/ remove object to be returned
1f855e9 to
d12a70f
Compare
We need lavamoat policies when the extension react deps are symlink to the app deps
Conscious ignoring popup-ui as it was causing issues due to imports from the extension submodule. And we are not using TS on the popup-ui yet.
matthewwalsh0
approved these changes
Mar 8, 2023
| done | ||
|
|
||
| rm -rf $EXTENSION_DIR/node_modules/@lavamoat/aa | ||
| ln -s ../../../../node_modules/@lavamoat/aa/ $EXTENSION_DIR/node_modules/@lavamoat/aa |
Member
There was a problem hiding this comment.
Not essential but we could combine all these into a single loop and remove the duplication if we create a list first with all the deps (react, webextension, and @lavamoat/aa).
vinistevam
approved these changes
Mar 8, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Add a desktop popup window. The main motivation for that is performance and an improved desktop experience. Why is it performant? Because on the desktop we can have the popup (approval window) loaded and ready since the moment that the app is initialised. We just hide or display and route accordingly whenever the popup needs to be displayed to the user.
Changes
Root
Two new environment variables:
Patch:
Init Extension submodule:
having more than one copy of react in the same appissue. (Also bear in mind that this is just required for the Desktop App.Pipeline
Updated the Circle pipeline, so that the Extension built for the e2e tests does not take into account the linked deps from the App. This allows the Extension to be built with lavamoat enabled and use the policies from the submodule.
App
Desktop popup
This is the actual desktop popup ui react app. It is loaded within an electron browserWindow. The way that it works is super easy.
UI
The ui is quite standard and mimics the structure of the desktop ui. The main differences are:
Build process:
To generate the bundle we have reused the same scripts that we use for desktop-ui. In fact we treated popup-ui as another entry point, but the commons are exactly the same. This means that most of the bundle is reused by both apps. Finally we have also split the css bundling. The dist will now include a
popup-index.cssfile that will be loaded by thepopup-ui.htmlfileConnection with Desktop App
It establishes a connection using the connectRemote but by passing our own duplex stream (called approvalStream). That approval stream is actually just a stream wrapping the popup-ui browser window webContents.send method. So to sum up we are:
Desktop App
Besides establishing the connection, the main process is also responsible for:
DESKTOP_POPUPenv var is set to true. It will load the popup-ui.html (or the dark version, depending on the current theme). And it will have its own context bridge (loaded using the popup-preload.js)