Simple payments/show messages response #7495

retrofox · 2017-07-20T15:18:17Z

This PR depending on #7409

This PR implements the ability to print the message when a transaction ends.

Testing

With Paypal sandbox account, click on the "Buy" button. After successful purchase, do you get a green success message? Try buying again. Does the message disappear when you click on the "buy" button again? Do you get the green success message after the second purchase?

artpi · 2017-07-21T12:00:58Z

I merged #7409 into feature/simple-payments branch. I am changing target of this PR to feature/simple-payments. Please rebase if need be.

iamtakashi · 2017-07-21T15:50:06Z

Just put together really quickly.

lamosty · 2017-07-21T17:19:28Z

@iamtakashi done here cbdb5fe. Thanks.

@iamtakashi

Props @iamtakashi for the suggestion.

retrofox · 2017-07-21T17:54:44Z

modules/simple-payments/paypal-express-checkout.js

@@ -9,6 +9,7 @@
 /* jshint unused:false */
 var PaypalExpressCheckout = {
 	sandbox: true,
+	$purchaseMessageContainer: null,


why are we using $ to define a var in JavaScript?

It's jQuery convention. If something is jQuery object, we use $ in the var name to state that. It helps with differentiating it from standard html element object so you can expect it to have jQuery capabilities.

retrofox · 2017-07-21T17:58:55Z

modules/simple-payments/paypal-express-checkout.js

+		if ( PaypalExpressCheckout.$purchaseMessageContainer.hasClass( 'show' ) ) {
+			PaypalExpressCheckout.$purchaseMessageContainer
+				.removeClass( 'show success error' )
+				.html( '' )


retrofox · 2017-07-21T18:37:54Z

When the page has more than one button the response is propagated into all of them.

retrofox · 2017-07-21T18:44:17Z

When a button is clicked it should clean only its own message.

retrofox · 2017-07-21T19:10:39Z

Before these changes when the transactions failed we were showing the error. Now it's silenced.

retrofox · 2017-07-21T20:07:24Z

Before these changes when the transactions failed we were showing the error. Now it's silenced.

This commit: 3fa3603

If the data sent to Paypal isn't consistent the request of the create endpoint will respond with some error which will be caught in the client-side.

iamtakashi · 2017-07-21T20:25:51Z

modules/simple-payments/simple-payments.css

@@ -0,0 +1,17 @@
+.jetpack-simple-payments__purchase-message {
+	display: none;
+	color: #fff;


Can we move only this color rule to a different block with a selector with a higher specificity so that it remains in white in most themes?

body .jetpack-simple-payments__wrapper p.jetpack-simple-payments__purchase-message { color: #fff; }

Adding this now

Just fyi I've changed to div instead of p;

body .jetpack-simple-payments__wrapper div.jetpack-simple-payments__purchase-message { color: #fff; }

Did it work? jetpack-simple-payments__purchase-message is a <p> isn't it? I've asked to change it to <p> since most themes should have some spacing for it so that we don't have to add some arbitrary margin.

I'm sorry Takashi. I didn't see your comment last night. It was tacked in this commit 59ea83a of the #7500

retrofox · 2017-07-21T20:45:26Z

modules/simple-payments/paypal-express-checkout.js

@@ -55,9 +55,6 @@ var PaypalExpressCheckout = {
 			throw new Error( 'PayPal module is required by PaypalExpressCheckout' );
 		}

-		// message DOM element instance


The idea here was create an instance of the button to use

iamtakashi · 2017-07-21T22:44:50Z

modules/simple-payments/simple-payments.css

+	padding: 1em;
+}
+
+body .jetpack-simple-payments__wrapper div.jetpack-simple-payments__purchase-message.show {


We don't need to add specificities to others. We only need the bloated selector for the <p>.

body .jetpack-simple-payments__wrapper p.jetpack-simple-payments__purchase-message { color: #fff; }

The rest should just be fine as they were.

Are we displaying links in the message?

The rest should just be fine as they were.

Let's rollback in #7500.

Are we displaying links in the message?

we were adding the emial address as a link. Let's cleaning it as well.

benhuberman · 2017-07-22T03:41:41Z

@jhnstn -- replying here to your P2 message re: successful purchase message. My main concern about the current version is that it's a bit repetitive and could be pared down just a bit. How about...

Thank you, [buyer name]! Your purchase was successful.
We just sent you an email with complete purchase details to [email address].

Alternatively, if the email can be considered a confirmation email, I'd just replace the second line with:

We just sent you a confirmation email to [email address].

Let me know if you'd like one of the editors to look at any other loose copy bits here. And for future reference, you can still ping us directly here with @Automattic/editorial (which we encourage people to do in repos where you can't add the needs copy review label).

jhnstn · 2017-07-22T05:21:07Z

This is working as described. Merging now and we can make any further changes in follow up PR's

artpi force-pushed the simple-payments/cpt branch from 893bbb3 to 5214211 Compare July 20, 2017 15:47

artpi changed the base branch from simple-payments/cpt to feature/simple-payments July 21, 2017 12:01

retrofox added 2 commits July 21, 2017 10:01

simple-payments: print message when the transaction ends.

37fddc3

simple-payments: clean message placeholder

b233443

retrofox force-pushed the simple-payments/show-messages-response branch from 65b9c15 to b233443 Compare July 21, 2017 13:01

lamosty added 2 commits July 21, 2017 18:57

Put the purchase message into HTML, hidden by default

26e96ea

Toggle purchase messages with either success/error and message

448d28f

Use p tag instead of div with margin

cbdb5fe

Props @iamtakashi for the suggestion.

lamosty force-pushed the simple-payments/show-messages-response branch from 5c02c3e to cbdb5fe Compare July 21, 2017 17:20

lamosty mentioned this pull request Jul 21, 2017

Simple payments styling #7500

Merged

lamosty added 5 commits July 21, 2017 19:21

Remove unneeded code

2246be7

Fix typo

93edd0b

Fix paypal checkout.js script error

ee85be4

Use .html instead of text to append html content

3dfa135

Removing one error message: there's no error to show, uneeded

3fa3603

lamosty changed the title ~~[WIP] Simple payments/show messages response~~ Simple payments/show messages response Jul 21, 2017

lamosty added the [Status] Needs Review This PR is ready for review. label Jul 21, 2017

Fix toggle logic of the purchase message

04a0822

retrofox commented Jul 21, 2017

View reviewed changes

iamtakashi reviewed Jul 21, 2017

View reviewed changes

retrofox commented Jul 21, 2017

View reviewed changes

simple-payments: improve showing transaction messages

fb41466

iamtakashi reviewed Jul 21, 2017

View reviewed changes

jhnstn added 2 commits July 21, 2017 21:37

fix linting issue

4966db8

update success copy

d839c7a

jhnstn removed the [Status] Needs Review This PR is ready for review. label Jul 22, 2017

jhnstn merged commit d5c7477 into feature/simple-payments Jul 22, 2017

jhnstn deleted the simple-payments/show-messages-response branch July 22, 2017 05:21

jeherve added the Simple Payments label Sep 7, 2017

jeherve added the [Feature] Pay with PayPal aka Simple Payments label Aug 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simple payments/show messages response #7495

Simple payments/show messages response #7495

retrofox commented Jul 20, 2017 •

edited

Loading

artpi commented Jul 21, 2017

iamtakashi commented Jul 21, 2017 •

edited

Loading

lamosty commented Jul 21, 2017 •

edited

Loading

retrofox Jul 21, 2017

lamosty Jul 21, 2017 •

edited

Loading

retrofox Jul 21, 2017

retrofox commented Jul 21, 2017

retrofox commented Jul 21, 2017

retrofox commented Jul 21, 2017

retrofox commented Jul 21, 2017

iamtakashi Jul 21, 2017 •

edited

Loading

retrofox Jul 21, 2017

retrofox Jul 21, 2017

iamtakashi Jul 21, 2017

retrofox Jul 22, 2017

retrofox Jul 21, 2017

iamtakashi Jul 21, 2017 •

edited

Loading

retrofox Jul 22, 2017

benhuberman commented Jul 22, 2017

jhnstn commented Jul 22, 2017

Simple payments/show messages response #7495

Simple payments/show messages response #7495

Conversation

retrofox commented Jul 20, 2017 • edited Loading

This PR depending on #7409

Testing

artpi commented Jul 21, 2017

iamtakashi commented Jul 21, 2017 • edited Loading

lamosty commented Jul 21, 2017 • edited Loading

Choose a reason for hiding this comment

lamosty Jul 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

retrofox commented Jul 21, 2017

retrofox commented Jul 21, 2017

retrofox commented Jul 21, 2017

retrofox commented Jul 21, 2017

iamtakashi Jul 21, 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

iamtakashi Jul 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benhuberman commented Jul 22, 2017

jhnstn commented Jul 22, 2017

retrofox commented Jul 20, 2017 •

edited

Loading

iamtakashi commented Jul 21, 2017 •

edited

Loading

lamosty commented Jul 21, 2017 •

edited

Loading

lamosty Jul 21, 2017 •

edited

Loading

iamtakashi Jul 21, 2017 •

edited

Loading

iamtakashi Jul 21, 2017 •

edited

Loading