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

Make Mini-cart settings independent from cart #775

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from
Open

Make Mini-cart settings independent from cart #775

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d45edcd
Modified PPEC Settings to add cart/mini-cart specific config.
achyuthajoy Jun 17, 2020
e1a4e45
Load Cart/Mini-cart specific settings on front-end
achyuthajoy Jun 19, 2020
b9c03cd
Modified JS to trigger collapse/expand of new settings sections
achyuthajoy Jun 19, 2020
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
57 changes: 39 additions & 18 deletions assets/js/wc-gateway-ppec-settings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,37 +267,57 @@
var display =
$( '#woocommerce_ppec_paypal_cart_checkout_enabled' ).is( ':checked' ) ||
( $( '#woocommerce_ppec_paypal_checkout_on_single_product_enabled' ).is( ':checked' ) && ! $( '#woocommerce_ppec_paypal_single_product_settings_toggle' ).is( ':checked' ) ) ||
( $( '#woocommerce_ppec_paypal_mark_enabled' ).is( ':checked' ) && ! $( '#woocommerce_ppec_paypal_mark_settings_toggle' ).is( ':checked' ) );
( $( '#woocommerce_ppec_paypal_mark_enabled' ).is( ':checked' ) && ! $( '#woocommerce_ppec_paypal_mark_settings_toggle' ).is( ':checked' ) ) ||
( $( '#woocommerce_ppec_paypal_cart_checkout_enabled').is( ':checked' ) && ! $( '#woocommerce_ppec_paypal_cart_settings_toggle' ).is( ':checked' ) ) ||
( $( '#woocommerce_ppec_paypal_mini_cart_checkout_enabled').is( ':checked' ) && ! $( '#woocommerce_ppec_paypal_mini_cart_settings_toggle' ).is( ':checked' ) );

$( '#woocommerce_ppec_paypal_button_layout, #woocommerce_ppec_paypal_button_size, #woocommerce_ppec_paypal_hide_funding_methods, #woocommerce_ppec_paypal_credit_enabled' ).closest( 'tr' ).toggle( display );
display && $( '#woocommerce_ppec_paypal_button_layout' ).change();
}

// Toggle mini-cart section based on whether checkout on cart page is enabled
$( '#woocommerce_ppec_paypal_cart_checkout_enabled' ).change( function( event ) {
if ( ! $( '#woocommerce_ppec_paypal_use_spb' ).is( ':checked' ) ) {
// Toggle cart section
$('#woocommerce_ppec_paypal_cart_checkout_enabled, #woocommerce_ppec_paypal_cart_settings_toggle').change(function (event) {
if ( ! $(' #woocommerce_ppec_paypal_use_spb' ).is( ':checked' ) ) {
return;
}

var checked = $( event.target ).is( ':checked' );
$( '#woocommerce_ppec_paypal_mini_cart_settings_toggle, .woocommerce_ppec_paypal_mini_cart' )
.closest( 'tr' )
.add( '#woocommerce_ppec_paypal_mini_cart_settings' ) // Select title.
.next( 'p' ) // Select description if present.
.addBack()
.toggle( checked );
checked && $( '#woocommerce_ppec_paypal_mini_cart_settings_toggle' ).change();
if ( ! $('#woocommerce_ppec_paypal_cart_checkout_enabled' ).is( ':checked' ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

You're missing a space before the argument to $() here.

// If cart page button is disabled, hide remaining settings in section.
$( '#woocommerce_ppec_paypal_cart_settings_toggle, .woocommerce_ppec_paypal_cart' ).closest( 'tr' ).hide();
} else if ( ! $( '#woocommerce_ppec_paypal_cart_settings_toggle' ).is( ':checked' ) ) {
// If cart page button is enabled but not configured to override global settings, hide remaining settings in section.
$( '#woocommerce_ppec_paypal_cart_settings_toggle' ).closest( 'tr' ).show();
$( '.woocommerce_ppec_paypal_cart, #woocommerce_ppec_paypal_cart_hide_funding_methods' ).closest( 'tr') .hide();
} else {
// Show all settings in section.
$( '#woocommerce_ppec_paypal_cart_settings_toggle, .woocommerce_ppec_paypal_cart' ).closest( 'tr' ).show();
$( '#woocommerce_ppec_paypal_cart_button_layout' ).change();
}
showHideDefaultButtonSettings();
} ).change();

$( '#woocommerce_ppec_paypal_mini_cart_settings_toggle' ).change( function( event ) {
// Only show settings specific to mini-cart if configured to override global settings.
var checked = $( event.target ).is( ':checked' );
$( '.woocommerce_ppec_paypal_mini_cart' ).closest( 'tr' ).toggle( checked );
checked && $( '#woocommerce_ppec_paypal_mini_cart_button_layout' ).change();
// Toggle Mini-cart section
$('#woocommerce_ppec_paypal_mini_cart_checkout_enabled, #woocommerce_ppec_paypal_mini_cart_settings_toggle').change(function (event) {
Copy link
Member

Choose a reason for hiding this comment

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

This section here is missing some spaces too. Sadly, our Travis checks don't check Javascript code, but here's the WP coding standards for JavaScript, so that you can take a look: https://developer.wordpress.org/coding-standards/wordpress-coding-standards/javascript/.

In terms of spacing, spaces are needed after opening ( and closing ) for function calls, and conditional blocks, etc.

if (!$(' #woocommerce_ppec_paypal_use_spb').is(':checked')) {
return;
}

if (!$('#woocommerce_ppec_paypal_mini_cart_checkout_enabled').is(':checked')) {
// If cart page button is disabled, hide remaining settings in section.
$('#woocommerce_ppec_paypal_mini_cart_settings_toggle, .woocommerce_ppec_paypal_mini_cart').closest('tr').hide();
} else if (!$('#woocommerce_ppec_paypal_mini_cart_settings_toggle').is(':checked')) {
// If cart page button is enabled but not configured to override global settings, hide remaining settings in section.
$('#woocommerce_ppec_paypal_mini_cart_settings_toggle').closest('tr').show();
$('.woocommerce_ppec_paypal_mini_cart, #woocommerce_ppec_paypal_mini_cart_hide_funding_methods').closest('tr').hide();
} else {
// Show all settings in section.
$('#woocommerce_ppec_paypal_mini_cart_settings_toggle, .woocommerce_ppec_paypal_mini_cart').closest('tr').show();
$('#woocommerce_ppec_paypal_mini_cart_button_layout').change();
}
showHideDefaultButtonSettings();
} ).change();
Copy link
Member

Choose a reason for hiding this comment

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

There's no need to remove the space here. See above.

}).change();

// Toggle Single Product section
Copy link
Member

Choose a reason for hiding this comment

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

Tip 💡: Nothing wrong with this comment, but in general, and unless they add necessary context, we try to avoid adding comments during PR as they can become distracting during code review. Also, comments should end with a dot "." 😸.

$( '#woocommerce_ppec_paypal_checkout_on_single_product_enabled, #woocommerce_ppec_paypal_single_product_settings_toggle' ).change( function( event ) {
if ( ! $( '#woocommerce_ppec_paypal_use_spb' ).is( ':checked' ) ) {
return;
Expand All @@ -318,6 +338,7 @@
showHideDefaultButtonSettings();
} ).change();

// Toggle Checkout Section
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

$( '#woocommerce_ppec_paypal_mark_enabled, #woocommerce_ppec_paypal_mark_settings_toggle' ).change( function() {
if ( ! $( '#woocommerce_ppec_paypal_use_spb' ).is( ':checked' ) ) {
return;
Expand Down
11 changes: 7 additions & 4 deletions includes/class-wc-gateway-ppec-cart-handler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ public function maybe_enqueue_checkout_js( $widget_title, $widget_instance = arr
if ( 'woocommerce_widget_cart' === $widget_id ) {
$gateways = WC()->payment_gateways->get_available_payment_gateways();
$settings = wc_gateway_ppec()->settings;
if ( isset( $gateways['ppec_paypal'] ) && 'yes' === $settings->cart_checkout_enabled && 'yes' === $settings->use_spb ) {
if ( isset( $gateways['ppec_paypal'] ) && 'yes' === $settings->mini_cart_checkout_enabled && 'yes' === $settings->use_spb ) {
wp_enqueue_script( 'wc-gateway-ppec-smart-payment-buttons' );
}
}
Expand Down Expand Up @@ -523,6 +523,8 @@ public function enqueue_scripts() {
if ( ! is_null( $page ) ) {
if ( 'product' === $page && 'yes' === $settings->single_product_settings_toggle ) {
$button_settings = $this->get_button_settings( $settings, 'single_product' );
} elseif ( 'cart' === $page && 'yes' === $settings->cart_settings_toggle ) {
$button_settings = $this->get_button_settings( $settings, 'cart' );
} elseif ( 'checkout' === $page && 'yes' === $settings->mark_settings_toggle ) {
$button_settings = $this->get_button_settings( $settings, 'mark' );
} else {
Expand All @@ -532,14 +534,15 @@ public function enqueue_scripts() {
$data = array_merge( $data, $button_settings );
}

// Load Mini Cart Settings if enabled
Copy link
Member

Choose a reason for hiding this comment

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

See above 👀.

$settings_toggle = 'yes' === $settings->mini_cart_settings_toggle;
$mini_cart_data = $this->get_button_settings( $settings, $settings_toggle ? 'mini_cart' : '' );
foreach ( $mini_cart_data as $key => $value ) {
unset( $mini_cart_data[ $key ] );
$mini_cart_data[ 'mini_cart_' . $key ] = $value;
}

Copy link
Member

Choose a reason for hiding this comment

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

Tip 💡: Similar to my comment about comments, adding or removing lines is something we try to avoid, unless it is to fix compliance with the standards. Added or removed blank lines make for a more complicated changeset or diff view with no real benefit.

$data = array_merge( $data, $mini_cart_data );

$data = apply_filters( 'woocommerce_paypal_express_checkout_payment_button_data', $data, $page );

if ( ! $settings->use_legacy_checkout_js() ) {
Expand Down Expand Up @@ -579,8 +582,8 @@ public function enqueue_scripts() {
* Adds the data-namespace attribute when enqueuing the PayPal SDK script
*
* @since 2.0.1
* @param string $tag
* @param string $handle
* @param string $tag
* @param string $handle
* @return string
*/
public function add_paypal_sdk_namespace_attribute( $tag, $handle ) {
Expand Down
43 changes: 38 additions & 5 deletions includes/settings/settings-ppec.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,9 +317,9 @@
),

'button_settings' => array(
'title' => __( 'Button Settings', 'woocommerce-gateway-paypal-express-checkout' ),
'title' => __( 'Global Button Settings', 'woocommerce-gateway-paypal-express-checkout' ),
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest we maintain the original wording. I totally understand what you mean here, but we would need to update screenshots and docs and maybe the difference could confuse some users.

'type' => 'title',
'description' => __( 'Customize the appearance of PayPal Checkout on your site.', 'woocommerce-gateway-paypal-express-checkout' ),
'description' => __( 'Customize the default appearance of PayPal Checkout on your site. If page specific customizations are not defined, these will be used by default. <br>For page specific customizations, use the configuration options inside <b>Cart / Mini-cart / Single Product / Regular Checkout</b>', 'woocommerce-gateway-paypal-express-checkout' ),
Copy link
Member

Choose a reason for hiding this comment

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

Same as above. For familiarity, maybe we can keep the original description. We could look into making the UI easier in another PR, just so that we can keep this one focused on separating the mini-cart buttons from the cart ones.

),
'use_spb' => array(
'title' => __( 'Smart Payment Buttons', 'woocommerce-gateway-paypal-express-checkout' ),
Expand Down Expand Up @@ -452,13 +452,22 @@
),
);

$per_context_settings['button_size']['class'] .= ' woocommerce_ppec_paypal_spb';
$per_context_settings['credit_enabled']['class'] .= ' woocommerce_ppec_paypal_spb';

/**
* Cart / global button settings.
* Default button settings.
Copy link
Member

Choose a reason for hiding this comment

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

Depending on what we decide to do, we might need to go back to the original text here. See my overall review for details.

*/
$settings = array_merge( $settings, $per_context_settings );

$per_context_settings['button_size']['class'] .= ' woocommerce_ppec_paypal_spb';
$per_context_settings['credit_enabled']['class'] .= ' woocommerce_ppec_paypal_spb';
/**
* Cart / global button settings.
*/
$settings['cart_settings'] = array(
'title' => __( 'Cart Button Settings', 'woocommerce-gateway-paypal-express-checkout' ),
'type' => 'title',
'class' => 'woocommerce_ppec_paypal_spb',
);

$settings['cart_checkout_enabled'] = array(
'title' => __( 'Checkout on cart page', 'woocommerce-gateway-paypal-express-checkout' ),
Expand All @@ -470,6 +479,20 @@
'default' => 'yes',
);

$settings['cart_settings_toggle'] = array(
'title' => __( 'Configure Settings', 'woocommerce-gateway-paypal-express-checkout' ),
'label' => __( 'Configure settings specific to cart', 'woocommerce-gateway-paypal-express-checkout' ),
'type' => 'checkbox',
'class' => 'woocommerce_ppec_paypal_spb woocommerce_ppec_paypal_visibility_toggle',
'default' => 'no',
'desc_tip' => true,
'description' => __( 'Optionally override global button settings above and configure buttons for this context.', 'woocommerce-gateway-paypal-express-checkout' ),
);
foreach ( $per_context_settings as $key => $value ) {
$value['class'] .= ' woocommerce_ppec_paypal_cart';
$settings[ 'cart_' . $key ] = $value;
}

/**
* Mini-cart button settings.
*/
Expand All @@ -479,6 +502,16 @@
'class' => 'woocommerce_ppec_paypal_spb',
);

$settings['mini_cart_checkout_enabled'] = array(
'title' => __( 'Checkout on Mini-cart', 'woocommerce-gateway-paypal-express-checkout' ),
'type' => 'checkbox',
'class' => 'woocommerce_ppec_paypal_visibility_toggle',
'label' => __( 'Enable PayPal Checkout on Mini-cart', 'woocommerce-gateway-paypal-express-checkout' ),
'description' => __( 'This shows or hides the PayPal Checkout button on the Mini-cart.', 'woocommerce-gateway-paypal-express-checkout' ),
'desc_tip' => true,
'default' => 'yes',
);

$settings['mini_cart_settings_toggle'] = array(
'title' => __( 'Configure Settings', 'woocommerce-gateway-paypal-express-checkout' ),
'label' => __( 'Configure settings specific to mini-cart', 'woocommerce-gateway-paypal-express-checkout' ),
Expand Down