Skip to content
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

Adds Wyre Widget #6434

Merged
merged 13 commits into from
Nov 4, 2019
Merged

Adds Wyre Widget #6434

merged 13 commits into from
Nov 4, 2019

Conversation

danjm
Copy link
Contributor

@danjm danjm commented Apr 10, 2019

fixes #5260

This PR adds the Wyre widget debit card widget to metamask for use as a crypto onramp.

Some notable aspects to this PR:

  • when a signature request is made by the wyre widget, metamask will show a new "Signature Request Modal", this will be layered above the widget

  • when we attempt to open the widget from the popup view, the extension is opened in full screen on the new DEPOSIT_ROUTE, which triggers an opening of the widget of the widget

  • This PR still needs a QA that goes all the way through wyre onboarding process. The following test data can be used:

101 Mission Street
San Francisco, CA 94105
Test Debit Card number:
Card: 9401113999999995
Exp: 10/2020
  • I guess we also need to load the script at run time, instead of copying the script code into our codebase... really I just wish it was an npm module

Peek 2019-04-10 11-53

@danjm danjm requested a review from whymarrh as a code owner April 10, 2019 14:36
@whymarrh whymarrh requested a review from tmashuang April 10, 2019 15:08
@whymarrh
Copy link
Contributor

@danjm if we're going to keep the Wyre script, we might be best to move it from ui/lib/wyre.js to ui/vendor/wyre.js as vendor/ is a bit more explicit than lib/. Thoughts?

@metamaskbot
Copy link
Collaborator

Builds ready [9409afe]: chrome, firefox, edge, opera

@danjm
Copy link
Contributor Author

danjm commented Apr 10, 2019

@whymarrh Good suggestion, I will do that, if we decide to keep the code in our codebase

Any thoughts on whether we should? My concern is that if we do we are subject to surprise breaking changes or subject our users to using outdated features/api/UX

I don't really want to load the code from a remote host at runtime either though...

@danjm danjm force-pushed the add-wyre-widget branch from 9409afe to 9595ea2 Compare April 12, 2019 15:52
@danjm
Copy link
Contributor Author

danjm commented Apr 12, 2019

@whymarrh In the latest commit I moved the wyre code to a vendor directory as you suggested.

@danjm danjm added the DO-NOT-MERGE Pull requests that should not be merged label Apr 12, 2019
@danjm danjm force-pushed the add-wyre-widget branch from 9595ea2 to fa7ee02 Compare April 12, 2019 15:59
@metamaskbot
Copy link
Collaborator

Builds ready [fa7ee02]: chrome, firefox, edge, opera

@tmashuang
Copy link
Contributor

Couldn't finish the flow, since the phone verification wasn't working.

@Chornuy91

This comment has been minimized.

@danjm danjm force-pushed the add-wyre-widget branch from fa7ee02 to 1a22f88 Compare July 18, 2019 16:18
@danjm danjm requested a review from Gudahtt as a code owner July 18, 2019 16:18
@danjm danjm force-pushed the add-wyre-widget branch from 1a22f88 to f7259e9 Compare July 22, 2019 10:47
@danjm danjm force-pushed the add-wyre-widget branch from f7259e9 to a6dfb60 Compare August 7, 2019 19:01
@metamaskbot
Copy link
Collaborator

Builds ready [f57d4a6]: chrome, firefox, edge, opera

@danjm danjm removed the DO-NOT-MERGE Pull requests that should not be merged label Aug 13, 2019
@danjm
Copy link
Contributor Author

danjm commented Aug 14, 2019

Here is a demo video: https://streamable.com/k66vl

@metamaskbot
Copy link
Collaborator

Builds ready [4476a17]: chrome, firefox, edge, opera

@bdresser
Copy link
Contributor

Sorry for the delay.

When I got to the end of the flow, I saw this error

Screen Shot 2019-08-14 at 2 10 53 PM

I never saw the "connect" screen at the beginning of the flow, I clicked "connect with MetaMask" and was immediately brought to the next step.

After clearing privacy data and trying again, I saw the connect popup and did not see this error on the final screen.

@metamaskbot
Copy link
Collaborator

Builds ready [5108750]: chrome, firefox, edge, opera

@kumavis
Copy link
Member

kumavis commented Sep 10, 2019

@danjm seems this has gone stale -- mostly ready?

@danjm danjm force-pushed the add-wyre-widget branch 2 times, most recently from f773588 to 81021f7 Compare September 18, 2019 16:50
@metamaskbot
Copy link
Collaborator

Builds ready [d6623a9]

@danjm danjm force-pushed the add-wyre-widget branch 3 times, most recently from f89aaee to a2d7e97 Compare November 3, 2019 19:54
@danjm danjm force-pushed the add-wyre-widget branch 2 times, most recently from 41fae32 to 00db455 Compare November 3, 2019 20:46
@metamaskbot
Copy link
Collaborator

Builds ready [00db455]

@metamaskbot
Copy link
Collaborator

Builds ready [9142f6c]

@danjm danjm added this to the UI Sprint 23 milestone Nov 4, 2019
Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@danjm danjm merged commit 6a4df0d into develop Nov 4, 2019
@whymarrh whymarrh deleted the add-wyre-widget branch November 4, 2019 17:25
tmashuang pushed a commit that referenced this pull request Nov 4, 2019
* Adds Wyre widget to the deposit modal.

* Move wyre widget code to vendor directory

* Get Wyre widget working without metamask connect/sign steps

* Code cleanup for wyre changes

* Change wyre widget to using prod environment

* Remove code allowing signing of wyre messages without confirmations

* Update wyre vendor code for wyre 2.0

* Remove unnecessary changes to provider approval constructor, triggerUI and openPopup

* Fix Wyre translation message

* Delete no longer used signature-request-modal

* Fix documentation of matches function in utils/util.js

* Code cleanup on wyre branch

* Remove front end code changes not needed to support wyre v2
tmashuang added a commit that referenced this pull request Nov 11, 2019
Gudahtt pushed a commit that referenced this pull request Nov 11, 2019
Gudahtt pushed a commit that referenced this pull request Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate Wyre for fiat onboarding
8 participants