Extension installation overlay #4423

jacogr · 2017-02-03T14:44:29Z

TODO: Test once extension is published with new/correct extension URL.

Displays application prompt for extension installation. (Closes https://github.com/ethcore/parity/issues/4300)

Based off #4412 with changes -

Doesn't display on hidden Web view, rather displays as part of application
Proper store set for backing the component, rather than loose-standing calls

Future work -

Create store & component tests
Integrate prompt with Home view once available

ngotchac · 2017-02-03T15:46:07Z

js/src/views/Application/Extension/extension.js

+  store = new Store();
+
+  render () {
+    const { shouldShowWarning } = this.store;


Why not showWarning ? Being polite is good though

ngotchac · 2017-02-03T15:46:50Z

js/src/views/Application/Extension/extension.js

+        <p>
+          <FormattedMessage
+            id='extension.intro'
+            defaultMessage='Parity now has an extension available for Chrome that allows the viewing of Ethereum identities and the safe browsing of Ethereum-enabled distributed applications. It is hightly recommended that you install this extension to further enhance your Parity experience.'


For now, the published extension won't have the augmentation part. Thus we can get rid of the viewing of Ethereum identities

ngotchac · 2017-02-03T15:48:52Z

js/src/views/Application/Extension/store.js

+
+  @action testInstall = () => {
+    this.shouldInstall = this.readStatus();
+    console.log('testInstall', this.shouldInstall);


Not sure if logging is needed here. We already have too many logs by default. If needed, could use the new logger feature

You are right, that one is a misplaced debugging comment. Thanks.

ngotchac · 2017-02-03T15:49:25Z

js/src/views/Application/Extension/store.js

+    const hasExtension = Symbol.for('parity.extension') in window;
+    const ua = browser.analyze(navigator.userAgent || '');
+
+    console.log('readStatus', hasExtension, ua);


Same as above

Same as above as well. Sorry.

ngotchac · 2017-02-03T15:59:47Z

js/src/views/Application/Extension/store.js

+      this.hideWarning(TEN_MINUTES + A_MINUTE);
+      setTimeout(() => {
+        this.testInstall();
+      }, TEN_MINUTES);


Shouldn't we have a way of closing the Warning only when the extension gets installed ? It seems bizarre to close it even if the installation failed (if the user clicked on install not on purpose, he can click on close).

Also, I'm not sure that the extension would actually be detected without a page refresh by just looking at the window object... To be verified

Yes, 100% - would need to test this, updating the URL now to run through.

ngotchac

Looks good. Still need to test when the extension is available

ngotchac · 2017-02-03T16:28:32Z

js/src/views/Application/Extension/extension.js

@@ -44,7 +44,7 @@ export default class Extension extends Component {
        <p>
          <FormattedMessage
            id='extension.intro'
-            defaultMessage='Parity now has an extension available for Chrome that allows the viewing of Ethereum identities and the safe browsing of Ethereum-enabled distributed applications. It is hightly recommended that you install this extension to further enhance your Parity experience.'
+            defaultMessage='Parity now has an extension available for Chrome that allows for the safe browsing of Ethereum-enabled distributed applications. It is hightly recommended that you install this extension to further enhance your Parity experience.'


"it is highly recommended"

could get rid of "allows for the safe"

Erk. Damnit.

ngotchac · 2017-02-03T17:36:51Z

Works fine, but I think we want to use the inline install : https://support.google.com/webmasters/answer/34592?hl=en

@tomusdrw Localhosts sites must be added to the extension config in the Chrome Store
There should be at least (127.0.0.1/localhost):300 and (127.0.0.1/localhost):8180

jacogr · 2017-02-03T17:42:10Z

*.web3.site is already added to webmaster tools. However 1.5.1 doesn't have full support for it (yet). (And even in those cases we are not yet re-directing people, and cannot redirect all)

Inline install does run through, obviously just pops up.

For 1.5.1, fine as it, works, the extra auto-browser window popup is not the end of the world.

ngotchac · 2017-02-03T17:49:03Z

Sure it's not the end of the world, but if it's only a one/few-line(s) to add to the config of the extension, I don't see why we shouldn't do it

jacogr · 2017-02-03T17:51:50Z

100%, will check once the beta is build in a short while, hopefully the config is updated and it is slightly smoother. As it stands, no reason not to go. (Unless that config change breaks the process in some way)

jacogr · 2017-02-03T19:01:44Z

Merging as-is so testing with beta build can be done.

* Extension installation overlay * Pr gumbles * Spelling * Update Chrome URL

jacogr · 2017-02-03T21:58:56Z

s/grumble/looksgood/ (non code related concerns)

* s/Delete Contract/Forget Contract/ (#4237) * Adjust the location of the signer snippet (#4155) * Additional building-block UI components (#4239) * Currency WIP * Expand tests * Pass className * Add QrCode * Export new components in ~/ui * s/this.props.netSymbol/netSymbol/ * Fix import case * ui/SectionList component (#4292) * array chunking utility * add SectionList component * Add TODOs to indicate possible future work * Add missing overlay style (as used in dapps at present) * Add a Playground for the UI Components (#4301) * Playground // WIP * Linting * Add Examples with code * CSS Linting * Linting * Add Connected Currency Symbol * 2015-2017 * 2015-2017 * 2015-2017 * 2015-2017 * 2015-2017 * 2015-2017 * 2015-2017 * Added `renderSymbol` tests * PR grumbles * Add Eth and Btc QRCode examples * 2015-2017 * Add tests for playground * Fixing tests * Split Dapp icon into ui/DappIcon (#4308) * Add QrCode & Copy to ShapeShift (#4322) * Extract CopyIcon to ~/ui/Icons * Add copy & QrCode address * Default size 4 * Add bitcoin: link * use protocol links applicable to coin exchanged * Remove .only * Display QrCode for accounts, addresses & contracts (#4329) * Allow Portal to be used as top-level modal (#4338) * Portal * Allow Portal to be used in as both top-level and popover * modal/popover variable naming * export Portal in ~/ui * Properly handle optional onKeyDown * Add simple Playground Example * Add proper event listener to Portal (#4359) * Display AccountCard name via IdentityName (#4235) * Fix signing (#4363) * Dapp Account Selection & Defaults (#4355) * Add parity_defaultAccount RPC (with subscription) (#4383) * Default Account selector in Signer overlay (#4375) * Typo, fixes #4271 (#4391) * Fix ParityBar account selection overflows (#4405) * Available Dapp selection alignment with Permissions (Portal) (#4374) * registry dapp: make lookup use lower case (#4409) * Dapps use defaultAccount instead of own selectors (#4386) * Poll for defaultAccount to update dapp & overlay subscriptions (#4417) * Poll for defaultAccount (Fixes #4413) * Fix nextTimeout on catch * Store timers * Re-enable default updates on change detection * Add block & timestamp conditions to Signer (#4411) * Extension installation overlay (#4423) * Extension installation overlay * Pr gumbles * Spelling * Update Chrome URL * Fix for non-included jsonrpc * Extend Portal component (as per Modal) #4392

Extension installation overlay

0a76e81

jacogr added A0-pleasereview 🤓 Pull request needs code review. M7-ui labels Feb 3, 2017

jacogr mentioned this pull request Feb 3, 2017

web browser dapp: prompt to install extension #4412

Closed

ngotchac reviewed Feb 3, 2017

View reviewed changes

ngotchac added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 3, 2017

Pr gumbles

19d421c

jacogr added A0-pleasereview 🤓 Pull request needs code review. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Feb 3, 2017

ngotchac reviewed Feb 3, 2017

View reviewed changes

jacogr added 2 commits February 3, 2017 18:07

Spelling

5767652

Update Chrome URL

a58d699

ngotchac added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Feb 3, 2017

ngotchac added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Feb 3, 2017

jacogr merged commit f48d8b0 into master Feb 3, 2017

jacogr deleted the jg-extension-prompt branch February 3, 2017 19:01

jacogr added a commit that referenced this pull request Feb 3, 2017

Extension installation overlay (#4423)

fa9204d

* Extension installation overlay * Pr gumbles * Spelling * Update Chrome URL

jacogr added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Feb 3, 2017

jacogr added the A8-looksgood 🦄 Pull request is reviewed well. label Feb 3, 2017

jacogr removed the A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. label Feb 3, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extension installation overlay #4423

Extension installation overlay #4423

jacogr commented Feb 3, 2017 •

edited

Loading

ngotchac Feb 3, 2017

ngotchac Feb 3, 2017

ngotchac Feb 3, 2017

jacogr Feb 3, 2017

ngotchac Feb 3, 2017

jacogr Feb 3, 2017

ngotchac Feb 3, 2017

jacogr Feb 3, 2017

ngotchac left a comment

ngotchac Feb 3, 2017 •

edited

Loading

jacogr Feb 3, 2017

ngotchac commented Feb 3, 2017

jacogr commented Feb 3, 2017 •

edited

Loading

ngotchac commented Feb 3, 2017

jacogr commented Feb 3, 2017

jacogr commented Feb 3, 2017

jacogr commented Feb 3, 2017

Extension installation overlay #4423

Extension installation overlay #4423

Conversation

jacogr commented Feb 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngotchac left a comment

Choose a reason for hiding this comment

ngotchac Feb 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngotchac commented Feb 3, 2017

jacogr commented Feb 3, 2017 • edited Loading

ngotchac commented Feb 3, 2017

jacogr commented Feb 3, 2017

jacogr commented Feb 3, 2017

jacogr commented Feb 3, 2017

jacogr commented Feb 3, 2017 •

edited

Loading

ngotchac Feb 3, 2017 •

edited

Loading

jacogr commented Feb 3, 2017 •

edited

Loading