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.

Sign up for GitHub

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

Jump to bottom

added handling of parameters of stripev3.load #35

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

added handling of parameters of stripev3.load #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a2c44d9
added handling of parameters of stripev3.load
caltabid Jun 2, 2018
8a971d9
update with SCA implementation from develop branch
caltabid Aug 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,7 @@ package.json.ember-try

.DS_Store
yarn-error.log

#Eclipse
Copy link
Contributor

Choose a reason for hiding this comment

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

This change isn't necessary. If it's the editor you use, then it should be in your global gitignore not here.

.settings
.project
23 changes: 21 additions & 2 deletions addon/components/stripe-element.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export default Component.extend({
// Fetch autofocus, set by user
let autofocus = get(this, 'autofocus');
let stripeElement = get(this, 'stripeElement');
let iframe = this.element.querySelector('iframe')
let iframe = this.element.querySelector('iframe');
if (autofocus && iframe) {
iframe.onload = () => {
stripeElement.focus();
Expand All @@ -64,6 +64,25 @@ export default Component.extend({

setEventListeners() {
let stripeElement = get(this, 'stripeElement');
stripeElement.on('ready', (event) => this.sendAction('ready', stripeElement, event));
Copy link
Contributor

Choose a reason for hiding this comment

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

The changes to this file need to be removed because they are unrelated to what this PR is trying to do - i.e. add options to the init method on the service.

stripeElement.on('blur', (event) => this.sendAction('blur', stripeElement, event));
stripeElement.on('focus', (event) => this.sendAction('focus', stripeElement, event));
stripeElement.on('change', (...args) => {
let [{ complete, error: stripeError }] = args;
this.sendAction('change', stripeElement, ...args);

if (complete) {
this.sendAction('complete', stripeElement);
} else if (stripeError) {
this.sendAction('error', stripeError);
}

set(this, 'stripeError', stripeError);
});
}

/*setEventListeners() {
let stripeElement = get(this, 'stripeElement');

stripeElement.on('ready', (event) => {
this._invokeAction('ready', stripeElement, event)
Expand Down Expand Up @@ -110,5 +129,5 @@ export default Component.extend({
focus() { },
change() { },
complete() { },
error() { }
error() { }*/
});
15 changes: 10 additions & 5 deletions addon/services/stripev3.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,13 @@ export default Service.extend({
this.configure();
}
},

load(publishableKey = null) {

unload() {
this.set('didConfigure', false);
this.set('didLoad', false);
Copy link
Contributor

Choose a reason for hiding this comment

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

In configure, the methods from the Stripe object are copied across to this. Totally agree with adding an unload method, in case you need to swap Stripe Connect accounts. However, I think unload should set all the methods that were copied across to null so that they are cleared out.

},

load(publishableKey = null, params = null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we set the params on this so that they are available in tests to assert that the correct params were passed through? Unlike publishableKey, I don't think they need to be wrapped in an if before setting, because no params are taken from default configuration.

if (publishableKey) {
this.set('publishableKey', publishableKey);
}
Expand All @@ -56,12 +61,12 @@ export default Service.extend({
let doLoad = shouldLoad ? loadScript("https://js.stripe.com/v3/") : resolve();

return doLoad.then(() => {
this.configure();
this.configure(params);
this.set('didLoad', true);
});
},

configure() {
configure(params) {
let didConfigure = this.get('didConfigure');

if (!didConfigure) {
Expand All @@ -71,7 +76,7 @@ export default Service.extend({
throw new EmberError("stripev3: Missing Stripe key, please set `ENV.stripe.publishableKey` in config.environment.js");
}

let stripe = new Stripe(publishableKey);
let stripe = new Stripe(publishableKey, params);
let functions = getProperties(stripe, STRIPE_FUNCTIONS);
setProperties(this, functions);

Expand Down