Simple payments styling #7500

artpi · 2017-07-21T16:29:55Z

Please continue

lamosty · 2017-07-21T16:48:27Z

modules/simple-payments/simple-payments.php

-	<div class='jetpack-simple-payments-price'>{$data['price']}</div>
-	{$items}
-	<div class='jetpack-simple-payments-button' id='{$data['dom_id']}_button'></div>
+	<div class='jetpack-simple-payments-payment-row'>


We should name the classes as jetapck-simple-payments__payment-row. At least that's what we do in Calypso and it's quite nice. Can we rework it?

We should avoid using underscores for front-end staff. https://make.wordpress.org/core/handbook/best-practices/coding-standards/css/#selectors

We should update the css class names according to this.

iamtakashi · 2017-07-21T16:55:27Z

If this was on Twenty Sixteen:

lamosty · 2017-07-21T17:20:44Z

Please merge #7495 before this PR since I've refactored class names.

iamtakashi · 2017-07-21T17:39:56Z

modules/simple-payments/simple-payments.php

@@ -94,9 +95,11 @@ function output_shortcode( $data ) {
 <div class='{$data[ 'class' ]} jetpack-simple-payments-wrapper'>
 	<div class='jetpack-simple-payments-title'>{$data['title']}</div>
 	<div class='jetpack-simple-payments-description'>{$data['description']}</div>


Can we have <p> inside of div.jetpack-simple-payments-description? Themes most likely has spacing for it and that would avoid having an arbitrary margin for this specific shortcode.

59ea83a9dfe3373ad56fb7d3cc457768f949b3dc

iamtakashi · 2017-07-21T17:41:01Z

modules/simple-payments/simple-payments.php

@@ -94,9 +95,11 @@ function output_shortcode( $data ) {
 <div class='{$data[ 'class' ]} jetpack-simple-payments-wrapper'>
 	<div class='jetpack-simple-payments-title'>{$data['title']}</div>


Can we have <strong> inside of div.jetpack-simple-payments-title ?

Actually, that^ was a bad idea. Sorry, let make it bold with CSS as it was.

iamtakashi · 2017-07-21T17:42:42Z

modules/simple-payments/simple-payments.php

-	{$items}
-	<div class='jetpack-simple-payments-button' id='{$data['dom_id']}_button'></div>
+	<div class='jetpack-simple-payments-payment-row'>
+		<div class='jetpack-simple-payments-price'>{$data['price']}</div>


Can we have also <p> inside of div.jetpack-simple-payments-price for the same reason?

Sorry, also <strong> please.

Ignore the <strong> suggestion, sorry!

59ea83a9dfe3373ad56fb7d3cc457768f949b3dc

retrofox · 2017-07-22T13:42:13Z

Please merge #7495 before this PR since I've refactored class names.

I'll change the class names according to this comment: #7500 (comment)

Also, we had to rebase it since there were conflicts.

retrofox · 2017-07-22T15:32:39Z

modules/simple-payments/simple-payments.css

@@ -16,6 +16,7 @@
 .jetpack-simple-payments-price {


Maybe you have some suggestion here @iamtakashi, using flexbox for instance. Feel free to edit this file.

retrofox · 2017-07-22T15:43:03Z

current state

https://retrofocs.wpsandbox.me/2017/07/22/workshop-planbook-post/

iamtakashi · 2017-07-22T23:15:11Z

modules/simple-payments/simple-payments.css

-body .jetpack-simple-payments__wrapper div.jetpack-simple-payments__purchase-message,
-body .jetpack-simple-payments__wrapper div.jetpack-simple-payments__purchase-message a {
+/* stronger rule in order to set the text color */
+body .jetpack-simple-payments-wrapper div.jetpack-simple-payments-purchase-message {


As per #7495, let's change it to <p> instead of <div>.

iamtakashi · 2017-07-22T23:22:07Z

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

 			style: {
 				label: 'pay',
-				color: 'blue'
+				shape: 'pill',


Just realised there is rect option for shape. I think I prefer the rectangle than the pill that feels a bit old school to me. Let's go with rect.

retrofox · 2017-07-22T23:31:38Z

iamtakashi · 2017-07-22T23:33:27Z

modules/simple-payments/simple-payments.php

+		return "
+			<div class='{$data['class']} ${cssPrefix}-wrapper'>
+				<p class='${cssPrefix}-purchase-message'></p>
+				<div class='${cssPrefix}-title'>{$data['title']}</div>


I think it's safer to have <p> inside of the <div> as well. This will likely add bottom margin to the title, but we actually need it. If the product doesn't have description there wouldn't be spacing between the title and the price.

<div class='${cssPrefix}-title'><p>{$data['title']}</p></div>

iamtakashi · 2017-07-22T23:37:05Z

modules/simple-payments/simple-payments.php

+			<div class='{$data['class']} ${cssPrefix}-wrapper'>
+				<p class='${cssPrefix}-purchase-message'></p>
+				<div class='${cssPrefix}-title'>{$data['title']}</div>
+				<p class='${cssPrefix}-description'>{$data['description']}</p>


It's good that we now have <p> around the price but can we keep the outer div we had?

<div class='${cssPrefix}-description'><p>{$data['price']}</p></div>

iamtakashi · 2017-07-22T23:37:51Z

modules/simple-payments/simple-payments.php

+				<div class='${cssPrefix}-title'>{$data['title']}</div>
+				<p class='${cssPrefix}-description'>{$data['description']}</p>
+				<div class='${cssPrefix}-purchase-box'>
+					<p class='${cssPrefix}-price'>{$data['price']}</p>


<div class='${cssPrefix}-price'><p>{$data['price']}</p></div>

iamtakashi · 2017-07-23T10:04:21Z

Please work on the image part ASAP. The style and markup can be tweaked after.

artpi · 2017-07-24T12:36:24Z

modules/simple-payments/simple-payments.php

@@ -12,6 +12,8 @@ class Jetpack_Simple_Payments {

 	static $shortcode = 'simple-payment';

+	static $cssClassnamePrefix = 'jetpack-simple-payments';


PHP is snake_case

artpi · 2017-07-24T12:37:43Z

modules/simple-payments/simple-payments.php

-";
-		return $output;
+
+		return "


This will make tabs in site html code as well, it was tabbed to the left so we dont print out unnecessary tabs

ic, I thought it was a mistake. Reverting now.

retrofox

LGTM. Feel free to merge before of after that #7503.

https://make.wordpress.org/core/handbook/best-practices/coding-standards/css/#selectors

#7500 (comment)

…yles.

lamosty · 2017-07-24T14:39:39Z

modules/simple-payments/simple-payments.php

-	<div class='jetpack-simple-payments__button' id='{$data['dom_id']}_button'></div>
+
+		return "
+<div class='{$data['class']} ${cssPrefix}-wrapper'>


I think adding the class name prefix as a variable is not very helpful. It's harder to read the styling then and I've never seen class naming in a variable. :) We are also not going to change the class name prefix I think -- why would we?

lamosty · 2017-07-24T14:40:58Z

modules/simple-payments/simple-payments.css

+	margin-top: 0.3em;
+}
+
+.jetpack-simple-payments-price {


We could use flexbox (like we do in Calypso Editor previews) or at least clear the floats in the parent div.

…kout properties"

iamtakashi · 2017-07-24T14:57:16Z

The QTY field still doesn't appear in my testing.

lamosty

artpi · 2017-07-24T15:15:07Z

I dont think that is what we want:
It is worse than state from 7e8d9ab

#7500 (comment)

@iamtakashi

* Introduce Jetpack_Simple_Payments class, load it * Add doc about format to CPT * Add more docs * Add enabled / disabled status * Add shortcode * reg shortcode * Add shortcode logic * Fix func * fix class * rename to product * Better content passing * Pass proper JS * Add comments * Rename to "paypal-express-checkout" * Add price * Label change * jshint fixes * filter * items * Throw error if paypal not defined * Info about script hosted with PP * Get rid of static keyword * Fix some lint issues * proper header * Adjust classes to WP coding standards * Change header * Change comment * add payload for number of items * Make ids work for paypal code * repair init code * Introduce ID for form * Add endpoint urls * Better domain * Endpoint urls with blog_id * Fix id * Add version * Change endpoint * Make sandbox * Add post meta syncing * Sandbox management * Review fixes * remove console Attempt to make test pass in JS * Fir for missing product * simple-payments: print message when the transaction ends. * simple-payments: clean message placeholder * simple-payments: implement styles to the block * some change * simple-payments: improve showing transaction messages * Put the purchase message into HTML, hidden by default * Toggle purchase messages with either success/error and message * Use p tag instead of div with margin Props @iamtakashi for the suggestion. * Remove unneeded code * Fix typo * Fix paypal checkout.js script error * Use .html instead of text to append html content * Removing one error message: there's no error to show, uneeded * Fix toggle logic of the purchase message * fix linting issue * update success copy * simple-payments: small coding improvements in output_shortcode() * simple-payments: do not use underscore in css class https://make.wordpress.org/core/handbook/best-practices/coding-standards/css/#selectors * simple-payments: minor improvements defining endpoints * simple-payments: use `<p>` tag for description and price #7500 (comment) * simple-payments: minor improvements * simple-payments: add purchase-box container. improve paypal button styles. * Simple Payments: update error message handling (#7503) - display success message from backend ( D6514-code ) - display all error messages * simple-pyaments: wrap all text into `<p>` elems * simple-payments: move globals as PaypalExpressCheckout properties * simple-payments: coding improvementes. * simple-payments: minor code improvements * rebase: squash -> "simple-payments: move globals as PaypalExpressCheckout properties" * Remove wpautop for Simple Payments product content/description (#7507) * Simple payments: image (#7508) Introduces an image to a payment button * Disable default sandboxing * simple-payments: user camel-case convention * reenable sandbox * Add env handling * Simple Payments: Style shortcode * simple-payments: inject msgs in the right place * simple-payments: show error in onAuthorize callback * fix additional error messages * Simple Payments: Modify message style * fix empty join call * Simple Payments: Reset iframe margin * wrap default error message * Implement review feedback * Add min field * SimplePayments: add formatted price meta data (#7521) Output `spay_formatted_price` if present * somple-payments: improve-error-handling * simple-payments: check processErrorMessage() param * Fixed lint errors. * Simple Payments: return wpcom blog_id if code is running on wpcom (#7524) * Simple Payments: add method to process success messages (#7528) Fixes styling of success messages.

artpi assigned lamosty, jhnstn and sirbrillig Jul 21, 2017

lamosty removed their assignment Jul 21, 2017

lamosty reviewed Jul 21, 2017

View reviewed changes

jhnstn unassigned sirbrillig Jul 21, 2017

iamtakashi reviewed Jul 21, 2017

View reviewed changes

retrofox mentioned this pull request Jul 22, 2017

Simple payments/show messages response #7495

Merged

retrofox force-pushed the simple-payments/styling branch from d0b7836 to 52a510b Compare July 22, 2017 13:39

retrofox force-pushed the simple-payments/styling branch 2 times, most recently from d4a24dd to 5f77a72 Compare July 22, 2017 14:33

retrofox self-assigned this Jul 22, 2017

retrofox reviewed Jul 22, 2017

View reviewed changes

iamtakashi reviewed Jul 22, 2017

View reviewed changes

retrofox force-pushed the simple-payments/styling branch from 3f0ef54 to 8c87368 Compare July 22, 2017 23:29

iamtakashi reviewed Jul 22, 2017

View reviewed changes

artpi commented Jul 24, 2017

View reviewed changes

retrofox approved these changes Jul 24, 2017

View reviewed changes

artpi and others added 9 commits July 24, 2017 11:08

simple-payments: implement styles to the block

e6509da

some change

7e8d9ab

simple-payments: small coding improvements in output_shortcode()

10d8026

simple-payments: do not use underscore in css class

4f4be62

https://make.wordpress.org/core/handbook/best-practices/coding-standards/css/#selectors

simple-payments: minor improvements defining endpoints

c2e0186

simple-payments: use <p> tag for description and price

4c2dae9

#7500 (comment)

simple-payments: minor improvements

43d30be

simple-payments: add purchase-box container. improve paypal button st…

912a9cb

…yles.

simple-pyaments: wrap all text into <p> elems

1102a9b

retrofox force-pushed the simple-payments/styling branch from 764f3b0 to 2ae1da0 Compare July 24, 2017 14:16

retrofox added 3 commits July 24, 2017 11:29

simple-payments: move globals as PaypalExpressCheckout properties

da337bb

simple-payments: coding improvementes.

40c4a34

simple-payments: minor code improvements

67a6324

retrofox force-pushed the simple-payments/styling branch from 6ea277d to 67a6324 Compare July 24, 2017 14:31

lamosty reviewed Jul 24, 2017

View reviewed changes

rebase: squash -> "simple-payments: move globals as PaypalExpressChec…

276a855

…kout properties"

lamosty suggested changes Jul 24, 2017

View reviewed changes

retrofox merged commit bcd211f into feature/simple-payments Jul 24, 2017

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

artpi pushed a commit that referenced this pull request Jul 24, 2017

simple-payments: use <p> tag for description and price

11cd68e

#7500 (comment)

jeherve added the Simple Payments label Sep 7, 2017

Viper007Bond deleted the simple-payments/styling branch January 29, 2018 23:18

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 styling #7500

Simple payments styling #7500

artpi commented Jul 21, 2017

lamosty Jul 21, 2017

iamtakashi Jul 22, 2017

retrofox Jul 22, 2017 •

edited

Loading

iamtakashi commented Jul 21, 2017 •

edited

Loading

lamosty commented Jul 21, 2017

iamtakashi Jul 21, 2017 •

edited

Loading

retrofox Jul 22, 2017

iamtakashi Jul 21, 2017 •

edited

Loading

iamtakashi Jul 22, 2017

iamtakashi Jul 21, 2017

iamtakashi Jul 21, 2017

iamtakashi Jul 22, 2017

retrofox Jul 22, 2017

retrofox commented Jul 22, 2017 •

edited

Loading

retrofox Jul 22, 2017

retrofox commented Jul 22, 2017 •

edited

Loading

iamtakashi Jul 22, 2017

iamtakashi Jul 22, 2017

retrofox commented Jul 22, 2017

iamtakashi Jul 22, 2017

iamtakashi Jul 22, 2017 •

edited

Loading

iamtakashi Jul 22, 2017

iamtakashi Jul 22, 2017 •

edited

Loading

iamtakashi commented Jul 23, 2017

artpi Jul 24, 2017

retrofox Jul 24, 2017

artpi Jul 24, 2017

retrofox Jul 24, 2017

retrofox left a comment •

edited

Loading

lamosty Jul 24, 2017

lamosty Jul 24, 2017

iamtakashi commented Jul 24, 2017

lamosty left a comment

artpi commented Jul 24, 2017

		@@ -94,9 +95,11 @@ function output_shortcode( $data ) {
		<div class='{$data[ 'class' ]} jetpack-simple-payments-wrapper'>
		<div class='jetpack-simple-payments-title'>{$data['title']}</div>

		@@ -12,6 +12,8 @@ class Jetpack_Simple_Payments {

		static $shortcode = 'simple-payment';

		static $cssClassnamePrefix = 'jetpack-simple-payments';

Simple payments styling #7500

Simple payments styling #7500

Conversation

artpi commented Jul 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

retrofox Jul 22, 2017 • edited Loading

Choose a reason for hiding this comment

iamtakashi commented Jul 21, 2017 • edited Loading

lamosty commented Jul 21, 2017

iamtakashi Jul 21, 2017 • edited Loading

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

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

retrofox commented Jul 22, 2017 • edited Loading

Choose a reason for hiding this comment

retrofox commented Jul 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

retrofox commented Jul 22, 2017

Choose a reason for hiding this comment

iamtakashi Jul 22, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iamtakashi Jul 22, 2017 • edited Loading

Choose a reason for hiding this comment

iamtakashi commented Jul 23, 2017

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

retrofox left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iamtakashi commented Jul 24, 2017

lamosty left a comment

Choose a reason for hiding this comment

artpi commented Jul 24, 2017

retrofox Jul 22, 2017 •

edited

Loading

iamtakashi commented Jul 21, 2017 •

edited

Loading

iamtakashi Jul 21, 2017 •

edited

Loading

iamtakashi Jul 21, 2017 •

edited

Loading

retrofox commented Jul 22, 2017 •

edited

Loading

retrofox commented Jul 22, 2017 •

edited

Loading

iamtakashi Jul 22, 2017 •

edited

Loading

iamtakashi Jul 22, 2017 •

edited

Loading

retrofox left a comment •

edited

Loading