Rough version of web3 support and embeddable ParityBar #16
Conversation
Conflicts: background/index.js content/index.js package.json
|
@ngotchac Ready to review and test (requires Parity build with |
Conflicts: background/index.js content/index.js package.json
| } | ||
|
|
||
| if (openedTabId) { | ||
| chrome.tabs.remove(openedTabId); |
There was a problem hiding this comment.
Doesn't it looks a bit sketchy to randomly open and close the tab when the Extension installs ? The user won't have the time to understand what happens ; I know I would be surprised. Might be better to have a settings/options/status window where it says that it needs the auth token, and on a button click it could open the right tab and gather the right data. However this might be included in a future issue.
There was a problem hiding this comment.
That tab is opened in the background (and it always opens only once), we also check if the node is up before opening it.
I think it might be better to just display a welcome page (that's actually quite a common thing for extensions) loaded from http://127.0.0.1:8180/#/welcome - it could list extension features, and the token would be extracted automatically :)
There was a problem hiding this comment.
Agreed with the welcome message, as per other extensions. Would log it in another issue though.
Because even if the tab is open in background, you can still see it open and close, right?
| styles: /styles\/embed\.([a-z0-9]{10})\.css/.exec(page), | ||
| scripts: /embed\.([a-z0-9]{10})\.js/.exec(page) | ||
| })) | ||
| .then(res => { |
There was a problem hiding this comment.
We might want to use a ServiceWorker here. It's its job to fetch and cache requests, and it's work pretty good. Also it might be good to have a way to export which files are required by the embed.html file, instead of using some RegExp. There might be a Webpack plugin/option for that
There was a problem hiding this comment.
AFAICT you cannot use ServiceWorker in background script of chrome extension (cause actually background scripts are more powerful then service workers). Also we cannot use service worker in the iframe, because of Mixed Content limitations.
So the way it works is:
Load an iframe with chrome-extension://<hash>/web3/embed.html and that page asks a background thread to fetch and provide correct files. (As I said ServiceWorker is not an option, cause the browser would detect that https had iframe that fetched non-https content and would block it as violating non-secure source.
There was a problem hiding this comment.
Ah ok, didn't know... So the API is not available either ?
| }) | ||
| .catch(err => { | ||
| console.error('Node seems down, will re-try', err); | ||
| setTimeout(() => extractToken(), 1000); |
There was a problem hiding this comment.
Might not want a background script making a request every second if I don't have my node up and running on installation. Cf. comment above
There was a problem hiding this comment.
Right we should use exponential backoff here.
There was a problem hiding this comment.
Updated to use exponential backoff everywhere.
| @@ -24,7 +24,7 @@ export default class Socials { | |||
| const { all } = Socials; | |||
|
|
|||
| let match = null; | |||
| const social = Object.values(all).find((social) => { | |||
| const social = Object.keys(all).map(key => all[key]).find((social) => { | |||
There was a problem hiding this comment.
Any reason not to use Object.values ?
There was a problem hiding this comment.
It's not supported in my Chrome and we don't include polyfill.
There was a problem hiding this comment.
Ah, I thought Babel was taking care of this... Should have a proper Webpack config I guess
|
Great start ! Some comments/remarks/questions. Seems to have some strange behaviors sometimes (eg. showing "Connecting to Parity..."), and tries to load some font file without success, or a service worker (that's from the UI code). |
Embeds ParityBar from running node.
Things left to do (as a separate issues)
web3.site?