Support MetaMask mobile #296
Conversation
|
|
|
Old New |
|
Hey @BrighTide! That looks really great! Would you do me a favour and rebase your PR on the current master? We'll then proceed on testing it |
|
Oh, sorry to be picky here but could you please rebase instead of merging upstream? We like to keep our git trees tidy :) |
|
Ha, don't worry I appreciate keeping things tidy. Just having a little trouble navigating rebasing from a fork. Good chance to read up on my gitfu |
|
I think it might even make sense to squash all your commits into one. |
filmerjarred
force-pushed
the
feature/288-support-metamask-mobile
branch
2 times, most recently
from
cd0dfc4 to
fc223aa
Compare
|
Ok, went through the rebase and git log seems to think my four commits are the most recent so I think we're good? |
Ah sorry just saw this, still want me to squash them? |
|
Ah, no, it's fine. Maybe just squash the commits in which the |
- pursuer-metamask was failing on mobile - Mobile version of metamask had etherum provider with new api - Re-implemented code to get the address and listen for account changes
filmerjarred
force-pushed
the
feature/288-support-metamask-mobile
branch
from
fc223aa to
ce051cb
Compare
.vscode/settings.json
Outdated
| @@ -0,0 +1,4 @@ | |||
| { | |||
| "typescript.validate.enable": false, | |||
There was a problem hiding this comment.
We would like to be agnostic about any IDE, so I'd appreciate if you didn't check in this file
There was a problem hiding this comment.
Taken care of 👍
package.json
Outdated
| "regenerator-runtime": "0.13.5" | ||
| "regenerator-runtime": "0.13.5", | ||
| "rimraf": "^3.0.2", | ||
| "cross-env": "^6.0.3" |
There was a problem hiding this comment.
You can actually use the newest version now. We upgraded node to v10.x
There was a problem hiding this comment.
Working as expected. Very nice work 💯
Here's some screenshots from running this on our QA server:
We've released an rc from your branch:
Calling the signMessage method:
The actual prompt from MetaMask Mobile:
The resulting message signature (Part of it hidden)
Thank you for your contribution @BrighTide !
| const ethereum = metamaskHelpers.getInpageProvider(); | ||
|
|
||
| if (ethereum.on) { | ||
| ethereum.on('accountsChanged', observer); |
| @@ -15,7 +16,7 @@ if (process.env.NODE_ENV === 'production') { | |||
| /* | |||
| * Clean the build folder before building | |||
| */ | |||
| const squeakyClean = buildFolder => run(`rm -rf ${buildFolder}`, {}, false); | |||
| const squeakyClean = buildFolder => rimraf.sync(buildFolder); | |||
There was a problem hiding this comment.
Yeah, right.
When I first set this up, I never gave any thoughts to non-unix systems
|
Hey @BrighTide, would you mind posting your address here, that you are using in the colony so that we can release your bounty once we've merged this? Cheers! |
|
No worries :) "0x5D748885F040d45439Bfe4e463b271E4180937D1" |
|
Fantastic! Merging and releasing the payout! |
Fix for #288 to support the new ethereum provider api
pursuer-metamask was failing on mobile
Mobile version of metamask was using an etherum provider with a new unsupported api
Re-implemented code to get the address and listen for account changes to respect new api whilst still being backwards compatible.
Added tests
Adjusted npm scripts to be windows compatible
The test environment for mobile can be found here https://github.com/BrighTide/purser/tree/69b9cfb685cc8c0dc3ad217ffb9c80653d61277f/metmask-mobile-test