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

Talk about features that would help Single Page Apps #29

Closed
jeffcarp opened this issue Mar 5, 2015 · 93 comments
Closed

Talk about features that would help Single Page Apps #29

jeffcarp opened this issue Mar 5, 2015 · 93 comments

Comments

@jeffcarp
Copy link

jeffcarp commented Mar 5, 2015

braintree-web's design comes from a time when checkout flows generally used a <form> POST. This is no longer the case in 2015, where SPAs and JS-heavy checkout flows are common.

There are a couple things that keep braintree-web from working well in SPAs, the biggest of which is that braintree.setup can only be called once per page load.

I'm starting this thread to discuss what kinds of features would be useful for making braintree-web work better in JS-heavy checkout flows.

Related Issues:

@jeffcarp jeffcarp changed the title Develop better support for Single Page Apps Talk about features that would help Single Page Apps Mar 6, 2015
@trainerbill
Copy link

I have actually had a lot of success using Angular and braintree-web. I didn't know that braintree-angular exists so I will need to take a look at that. A couple things I would recommend are:

  1. paymentMethodNonceReceived: function (event, nonce). It would be nice if this function returned a creditcard object containing like last 4 of card etc so that we do not need to vault the customer/method to display it on a confirmation route. Obviously no full informaiton for PCI. Similar to how the paypal button returns shipping and email info
  2. Obviously the event handlers on forms have been a problem. I have my braintree.setup calls in a service and it seems like I can use JQuery to empty() the drop in container and call it again. Would be nice to see this reworked so that we can call setup multiple times without any work arounds.
  3. Need a bower install for braintree-data. Also need a callback function on in the BraintreeData.setup call so that it returns the data instead of creating a DOM element in the form.

There are more that I ran into so I will update as I run into them.

@kyledetella
Copy link
Member

@trainerbill regarding paymentMethodNonceReceived, as of v2.5.0 we added a new root level callback called onPaymentMethodReceived which returns more details for Credit Card transactions. Unfortunately, we have yet to release the official documentation, but you can read more about the callback here and I am happy to answer more any further questions you may have.

@trainerbill
Copy link

Awesome! Thanks for the heads up on that.

@tnunes
Copy link

tnunes commented May 19, 2015

👍 for a quick resolution of this. These issues make it really difficult to integrate braintree in a SPA.

@filipediasf
Copy link

We appreciate a quick fix to ease integration.

@joanamelo
Copy link

👍

1 similar comment
@zebateira
Copy link

👍

@tnunes
Copy link

tnunes commented May 20, 2015

We were experiencing an issue where loading the braintree widget (with braintree.setup()) for a second time on the same single page app (after removing the previous one, but without reloading the page), would make the widget use the client token with which braintree.setup() was called for the first time, effectively giving access to the vault of the first user.

This is a serious issue, since it could allow a user A access to the vault of a user B after user A logged out, if the page was not refreshed meanwhile.

After investigating the problem more thoroughly, we were able to work around the issue, but we had to resort to some nasty hacks and fragile solutions that might break with future releases of braintree-web.

We found that all the following steps are required for a proper cleanup and new setup of the widget:

  • remove the DOM element where the widget is attached;
  • remove the "braintree-dropin-modal-frame” iframe added to the body;
  • delete window.Braintree object;
  • delete various window.callback_json_xxxx functions (2 per setup());
  • remove various “on message” event listeners attached to the window object (5 per setup());
  • remove the script from the page (since we use require.js, we’re resorting to requirejs.undef('braintree’) to do this).

After all these steps are done, we can then require the braintree.js script again and invoke braintree.setup() with different parameters and user vaults and payments work as expected.

To remove the event listeners left hanging in the window object we resorted to monkey-patching the window.addEventListener method, which feels wrong and may lead to other issues.

A destroy() method implemented by braintree-web has the benefits that it doesn’t need to resort to nasty hacks and monkey-patches to keep track of callbacks and event handlers that are registered globally, since the module can keep track of its own references for easier removal.

If you could provide a braintree.destroy() (or similar) method that would do the steps described, integration in single page applications would be much easier and we wouldn’t have to fear that future upgrades of braintree-web could break our workaround solution.

Reloading of braintree.js should be left to consumers, since the method to do that depends on how the script is included, but all the other steps could be provided by your own cleanup mechanism.

@imgntn
Copy link

imgntn commented May 28, 2015

wow @tnunes i was trying the same kind of workaround a couple of days ago, but missed the callbacks and event listeners on the window. thanks for the info. do you think the hosted fields would have the same issues?

@tnunes
Copy link

tnunes commented May 28, 2015

Hi @imgntn,

I haven't used hosted fields, so I can't help you that. I know, however, that braintree-data also presents the same kind of issues, since to integrate it I also had to find out and clean what it leaves attached to the page.

@ms7s
Copy link

ms7s commented Jun 1, 2015

Since github doesn't support voting for issues, I'm adding a comment: Please add braintree.destroy().

@karpitsky
Copy link

+1

@niklr
Copy link

niklr commented Jun 6, 2015

We NEED braintree.destroy()!

@fanfreeman
Copy link

Really need this. Stripe has it, but we don't want to give up on Braintree just because of this issue.

@baillyje
Copy link

+1

@tnunes, Do you know if it's possible to target the braintree 'on message' events listener without deleting all others ? Any Ideas of their name ?

@tnunes
Copy link

tnunes commented Jun 11, 2015

@baillyje, what I do is monkey-patch the window.addEventListener function right before I setup braintree.

In my implementation I accumulate all event listeners in an array before proxying to the real window.addEventListener.

Then, in my braintree.destroy, I use window.removeEventListener to remove all listeners I previously saved references to, right before I restore the original window.addEventListener function.

This is hackish and fragile, but as long as nothing aside from braintree adds listeners to the page from the moment you call setup until you destroy the widget, you should be good.

@HiroAgustin
Copy link

We have the same problem @tnunes described, thank you for the hack. Still, I think we are going to do an full page reload until braintree.destroy (or something similar) is implemented by default.

@tyleriguchi
Copy link

Glad I stumbled upon this issue, was tearing my hair out trying to figure out why I was getting errors in my Ember app when I tried to add and remove paypal accounts. A teardown method would really help out for single page apps.

@schmitzdenis
Copy link

+1 need Braintree.destroy or at least Braintree.update, in order for Braintree.setup to be only called once and prevent unnecessary iframes and event listeners from spawning.

@paulrnash
Copy link

+1 come on braintree, we're all waiting!

@ThoamsClark
Copy link

+1

Would be really helpful. Reloading the whole page seriously detracts from our whole checkout experience.

@tastyeggs
Copy link

+1

Come on braintree, get your act together. Come live in the future with us. Everyone is designing single-page applications and it is extremely frustrating when we have to integrate with libraries designed by companies who think they use the latest tech, but actually have no clue.

This is CS 101 fundamentals; make your code self-contained; make it leave no trace on the system its running on. We thought we should use braintree because it would be easier to integrate, but it's taking the same amount to time, and same amount of nasty hacks as it takes to integrate with your parent company.

You sitting on this issue for 3 months shows that you either don't care about your users experience, or you have no clue what single-page applications are about, hence don't see how badly you're crippling the experience of our users and developers. Either way, it reflects poorly on your company.

Forgive my strong language; but this is exactly how I feel. But now we're all trapped to writing shitty code, because either you guys don't care, or you guys just don't get it.

Wake up!!

@paulrnash
Copy link

Pretty much what he said. I've wasted hours and hours trying to make
something reasonable out of this technology. It doesn't seem like either
the default web integration or hosted fields will work for anything but a
multiple page click through experience. After earlier experiences where
braintree's support had been impeccably good, I am now finding that it is
not so responsive and technically they are not keeping up with the times.
Perhaps all the PayPal and eBay split noise is affecting the focus of the
team or confusing the priorities. I am seriously considering switching to a
competitor after my experience dealing with this mess, particularly sense
the communication from Braintree has mostly been either radio silence or
"uh sorry." they don't even seem to be aware of the reason this is such an
inconvenience.
On Jun 29, 2015 3:01 PM, "tastyeggs" notifications@github.com wrote:

+1

Come on braintree, get your act together. Come live in the future with us.
Everyone is designing single-page applications and it is extremely
frustrating when we have to integrate with libraries designed by companies
who think they use the latest tech, but actually have no clue.

This is CS 101 fundamentals; make your code self-contained; make it leave
no trace on the system its running on. We thought we should use braintree
because it would be easier to integrate, but it's taking the same amount to
time, and same amount of nasty hacks as it takes to integrate with your
parent company.

You sitting on this issue for 3 months shows that you either don't care
about your users experience, or you have no clue what single-page
applications are about, hence don't see how badly you're crippling the
experience of our users and developers. Either way, it reflects poorly on
your company.

Forgive my strong language; but this is exactly how I feel. If this
were an open-source library, I would have fixed this by now. But now we're
all trapped to writing shitty code, because either you guys don't care, or
you guys just don't get it.

Wake up!!


Reply to this email directly or view it on GitHub
#29 (comment)
.

@rturumella
Copy link

Hey folks - I'm a Product Manager with the Braintree SDK and Developer Experience team.

We understand your frustration and the importance of providing an experience that works in a myriad of integration scenarios, including Single Page Applications.

We also recognize that this is an area in which we can improve our SDK. We're currently actively exploring the features we need to add and making considerations to preserve compatibility for all braintree.js users.

This is our best path forward for us to provide a more ideal and complete experience for all of our merchants. I'll keep you posted on the progress of this as I know that it is currently resulting in a less than ideal integration experience.

In the meantime, take a look at the docs for our advanced integration or feel free to contact me (rohit@getbraintree.com) if you have further questions or concerns.

@tastyeggs
Copy link

@rturumella There's no information in your documentation on how we can do custom integration with PayPal, other than this one line here:

https://developers.braintreepayments.com/javascript+dotnet/guides/paypal/overview

"Braintree offers a few options when accepting PayPal payments. First, you'll need to choose whether you would like to use our Drop-in UI or a custom integration. Learn more about differences between these integrations".

@tastyeggs
Copy link

@rturumella Any chance you guys can publish the un-minified source of the JS client? Might just make it easier for us to workaround this issue.

@wmadden
Copy link

wmadden commented Sep 1, 2015

@rturumella thanks very much for the explanation.

FWIW the worst part of this for us - the scariest part - was when we
realized that because that nonce was still present even after the user had
logged out, any other person could have used our app to make purchases with
the previous user's payment method if they got ahold of the device.

On 1 September 2015 at 01:56, rturumella notifications@github.com wrote:

@shermandickman https://github.com/shermandickman The .destroy() (or
similarly named API) functionality is coming next week. We’ll be pushing it
as an edge branch so you all can at least kick the tires and see progress.
Thanks for your patience.


Reply to this email directly or view it on GitHub
#29 (comment)
.

@kyledetella
Copy link
Member

As @rturumella mentioned last week, we are delivering a beta build of braintree.js that exposes the ability to tear down an integration and re-create a new one during a lifecycle of a single page view.

Right now this new version is deployed as a beta build and can be downloaded from here:

We will be adding full documentation in an accompanying production release in the coming weeks. In the meantime, we would love to take in some feedback on this new functionality. We know how important this is and we want to make sure when it goes out in production that the experience is rock solid.

Since version 2.7.0, the configuration object passed during braintree.setup has provided an onReady callback. As of this latest build the onReady callback will provide an integration object on which you can perform actions such as teardown. A basic example of this looks like:

var myIntegration;

braintree.setup(TOKEN, 'dropin', {
  onReady: function (integration) {
    myIntegration = integration;
  }
});

// When you're ready to destroy your integration
myIntegration.teardown(function () {
  // braintree.setup can safely be run again!
});

This teardown method will go ahead and do the work necessary to destroy anything that braintree has created on your page including iframes and hidden input elements. It will also close any popups or modals that have been opened by our integration (PayPal). teardown will execute a callback when these operations have completed and you can safely proceed with your next steps.

@berendberendsen
Copy link

@kyledetella @rturumella just tested this and it seems to be working fine. I added a $scope.$on('$destroy' bit in the code to call the teardown() and it seems to have nicely cleaned up the dropin. Thanks for the resolution in the end!

@cityreader
Copy link

@rturumella Does this update also apply to hosted fields js?

@kyledetella
Copy link
Member

@cityreader this update will apply to hosted-fields but that has not been released yet. We are aggressively working to provide teardown for hosted-fields integrations as well. That work lives in a separate beta build and there are a few things we need to reconcile first. I will keep you posted with updates on that.

@theotherzach
Copy link

@kyledetella How are we doing on the hosted fields integration?

@kyledetella
Copy link
Member

@theotherzach the teardown functionality in hosted-fields will be available within 2 weeks. Again, we will make sure to drop the news here as soon as its ready.

Have you had a chance to try out teardown in your Drop-in integration? Is it working as expected for you?

@rbsmidt
Copy link

rbsmidt commented Sep 11, 2015

@kyledetella sounds good - looking forward to the new beta.
Just to clarify: I'm currently testing out hosted fields on a React JS powered check out flow. I'm currently having a hard time getting my braintree.setup to fetch my form correct. I do get the onReady-callback (but with no arguments passed - guessing it's because the integration-argument isn't included in the newest hosted-fields beta build?). But for instance my placeholders and other options aren't applied to the input fields in question. Also the onFieldEvent isn't firing when changing value in input fields. When i try to submit a transaction, my onError-event fires with this msg:

message: "User did not enter a payment method"
type: "VALIDATION"

Also my onFieldEvent fires for all inputs on submit - but just after the error msg. They all appear to be empty according to the onFieldEvent, and the container is ".braintree-hosted-fields-invalid" - which obvously tells me something is wrong with my braintree init.

I'm guessing the above is caused by the way react is rendering js and markup. Will i be able to control this better, when the teardown-function is added to braintree?
And is it correct understood, that with the current build of hosted fields, it is not possible to load the hosted fields dynamically on a onpage javascript application?

Thanks for keeping us all updated through development!

@kyledetella
Copy link
Member

@rbsmidt the onReady callback will not pass any arguments until teardown is introduced into Hosted Fields, so that is expected.

As for the other issues you mentioned (placeholders and options not being applied), these sound unrelated to teardown. It will be easier for us to debug and help you out if we could get some more context on your integration. Would you be willing to post snippets of your setup? If you would rather not post them here, send us an email to hosted-fields@getbraintree.com and we can take this conversation there.

Regarding your last question, let me make sure I understand, are you asking if it is possible to setup hosted fields dynamically after page load? If so, yes you are able to do that. If I am misunderstanding the question, let me know.

@rbsmidt
Copy link

rbsmidt commented Sep 11, 2015

Thanks for your reply @kyledetella

Yes i've been debugging a bit more, and it seems that you are right, my issue lies elsewhere. I'm happy to provide an example:

var Checkout = React.createClass({
  componentDidMount: function(){
    console.log('checkout mounted..');
    this.initPayment();
  },
  initPayment: function(){
    console.log('init payment', this);
    var options = {
      id: "payment-form",
      hostedFields: {
        onFieldEvent: function(event){
          console.log('field event triggerede', event);
        },
        number: {
          selector: '#card-number',
          placeholder: 'Kortnummer'
        },
        cvv: {
          selector: '#cvv',
          placeholder: 'CVV'
        },
        expirationDate: {
          selector: '#expiration-date',
          placeholder: 'Kortnummer'
        }
      },
      onReady: function(){
        console.log('braintree ready');
      },
      onError: function(error){
        console.log('an error occurred', error);
      },
      onPaymentMethodReceived: function(event, data){
        console.log('event', event, 'data', data);
      }
    }
    braintree.setup(this.props.state.token, "custom", options);
  },
  render: function(){
    console.log('render checkout', this);
    return(
        <div className="grid-checkout">
          <h3>Check ud</h3>
          <form id="payment-form">
              <input type="text" name="firstname" id="fistname" placeholder="Fornavn" />
              <input type="text" name="lastname" id="lastname" placeholder="Efternavn" />
              <input type="text" name="email" id="email" placeholder="Email" />
              <input type="text" name="tlf" id="tlf" placeholder="Telefon" />
              <p><img src="/images/icon_security.svg" height="15" width="auto" /> Sikker betaling <img src="/images/cc@2x.png" width="auto" height="25"/></p>
              <input type="text" name="card-number" id="card-number"  />
              <input type="text" name="cvv" id="cvv" />
              <input type="text" name="expiration-date" id="expiration-date" />
              <button>Betal</button>
          </form>
        </div>
    );
  }
});

As stated in my last comment, i the onReady-event fires successfully after componentDidMount and the initPayment, but my placeholders doesn't show up, and the onFieldEvent doesn't fire either. When i press the submit-button, my three hosted fields gets the class braintree-hosted-fields-invalid. But before i press submit, the input-fields doesn't have a class. So it seems to me, that my braintree setup never "catches" the three input fields - until i try submitting, and somehow they are registered as empty fields by braintree.js.
The error message is still as stated in my last comment.

Any idea what could cause this setup issue?

Best regards,
Rasmus

@lilaconlee
Copy link

@rbsmidt It looks like if you replace the input elements for card-number, cvv, and expiration-date with divs, everything works. Hosted Fields actually uses these div elements as containers in which we inject iframes which contain the "hosted fields". Take a look at the documentation and/or this blog post for more detailed information on setting up Hosted Fields.

Here is an example of the updated code for you to try out:

...
  render: function(){
    console.log('render checkout', this);
    return(
        <div className="grid-checkout">
          <h3>Check ud</h3>
          <form id="payment-form">
              <input type="text" name="firstname" id="fistname" placeholder="Fornavn" />
              <input type="text" name="lastname" id="lastname" placeholder="Efternavn" />
              <input type="text" name="email" id="email" placeholder="Email" />
              <input type="text" name="tlf" id="tlf" placeholder="Telefon" />
              <p><img src="/images/icon_security.svg" height="15" width="auto" /> Sikker betaling <img src="/images/cc@2x.png" width="auto" height="25"/></p>

              <div id="card-number"></div>
              <div id="cvv"></div>
              <div id="expiration-date"></div>

              <button>Betal</button>
          </form>
        </div>
    );
  }
});
...

I hope this helps you out. Let us know if you have any more questions.

@rbsmidt
Copy link

rbsmidt commented Sep 11, 2015

@lilaconlee Ahh you are totally right. I tested with divs instead of inputs, and it worked like a charm! Thanks a bunch.

@theotherzach
Copy link

@kyledetella we needed to change our drop-in solution out for hosted fields because of styling concerns.

@davidbryant
Copy link

Hi,

I've just been playing with the beta version and found that the onReady callback is not called in some cases, meaning that you aren't provided with the teardown function to clean it up. Specifically if the form is hidden at the time that setup() is called, onReady is not called until you that form becomes visible. If it never becomes visible then you never get the callback so cannot unregister the event handlers.

I've run into this because I have a multi-step check out process with the dropin UI on the second page, so I was loading it in the background since it can take a while to load.

Is this intentional? If so why?

Cheers,
Dave

@kyledetella
Copy link
Member

@davidbryant thanks for bringing this up. You are correct that onReady will not fire in the case of Drop-in until the UI has fully rendered (including the completion of css transitions). When the container/form for Drop-in is hidden, that action will be delayed until the surrounding DOM is made visible.

This is by design since the integration is technically not "ready" until it has fully rendered and is capable of tokenizing payment methods.

The concept of wanting to "preload" the Drop-in is interesting and I am glad you brought it up. The semantics of onReady suit the original intention of the Drop-in UI, but hearing about interesting use cases like yours really help as we make considerations for our products and features. I will bring this issue up with the team and let you know where we end up.

To be honest though, it is unlikely we will change this pattern anytime soon, so in the case of your UI, I would recommend initializing Drop-in only after you have rendered the containing view and tearing it down prior to un-rendering that view.

@davidbryant
Copy link

So this would also imply that I need to prevent all navigation within a SPA from the time that I call braintree setup() until it fires its onReady event? Otherwise there's a chance that a navigation event in that timeframe will prevent onReady from being fired and so not provide me with the teardown callback?

@mrak
Copy link
Contributor

mrak commented Sep 14, 2015

@davidbryant that is correct.

It's not a convenient restriction, but in integrations such as dropin or hosted-fields there is a lot of cross-domain machinery being asynchronously set in motion; cutting it off halfway through with a navigation event could lead to dangling DOM nodes, event handlers, or an unusable integration.

@kyledetella
Copy link
Member

This afternoon version 2.14.0 was released. As you will see in the CHANGELOG, this adds Hosted Fields with teardown.

This thread was originally opened to engage a dialogue on this matter and it certainly served that purpose. Should you experience any specific issues with teardown or Hosted Fields, please open a separate issue so that we can address them accordingly.

@cheyner
Copy link

cheyner commented Sep 17, 2015

@kyledetella how should we handle BraintreeData.setup() when using teardown()?

Right now I'm calling

braintree.setup(this.props.paymentToken, "custom", {
  onReady: function(integration) {
    Checkout.braintreeIntegration = integration;
  },
  id: "payment-method-form",
  hostedFields: {...});
BraintreeData.setup(this.props.merchantId, 'payment-method-form', 'production');

And then later using Checkout.braintreeIntegration.teardown() as needed. It's working well, but will there be complications if I'm repeatedly calling BraintreeData.setup()?

@kyledetella
Copy link
Member

@cheyner it doesn't sound as though you are experiencing issues when repeatedly calling BraintreeData.setup() so it should be fine. However, should you encounter any hiccups, feel free to open a new issue about this and we'll take a look.

@bezhermoso
Copy link

@kyledetella Thank you, the teardown APi is working great for us.

Just one bug I encountered: the .bt-overlay doesn't seem to be removed after teardown, and an new one is created whenever the PayPal login screen is displayed. This results to the transluscent black overlay to get darker and darker every teardown/initialization cycle. I remedied it by removing .bt-overlay within the teardown callback. Not sure if this is happening to others, too.

@kyledetella
Copy link
Member

@bezhermoso glad to hear teardown is working out!

Would you mind opening a separate issue regarding .bt-overlay not being removed? It will be easier to track and handle specific issues in more focused threads. In the meantime, we will take a look.

@kyledetella
Copy link
Member

The primary concern for most participants in this thread has been addressed in last week's 2.14.0 release. In the interest of keeping the discussion as specific as possible, we're going to close this issue.

Please open new issues for any further Single Page App concerns or questions. Thanks!

@trainerbill
Copy link

Thanks for the functionality. When will it be documented? Can you now call .setup() then braintree.teardown() and .setup again?

@kyledetella
Copy link
Member

@trainerbill we will provide full documentation once this release is hotlinked. That should happen sometime this week.

For now, the explanation and code provided above remains accurate.

Can you now call .setup() then braintree.teardown() and .setup again?

Yes, that is a core allowance with this new feature!

@parreirat
Copy link

Some months ago we badly needed this teardown() feature. We were changing billing just now and it's great that it's now available! Thank you!

For those looking for the documentation:
https://developers.braintreepayments.com/guides/client-sdk/javascript/v2#teardown

@kyledetella
Copy link
Member

@parreirat glad to hear it helped out!

Also, good call on posting the documentation in this thread, that slipped my mind!

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

No branches or pull requests