search

Arsenal21 / stripe-payments

Stripe Payments Plugin for WordPress
11 stars 8 forks source link

Add option "Compatability with page builders" #97

Closed erommel closed 5 years ago

erommel commented 5 years ago

Currently, compat_mode="1" shortcode parameter is used for this. However, it's not enough. There have been several issues reports from users who are using various page builders. I have investigated those issues and figured some page builders are outputting content to frontend while being in admin mode. E.g. is_admin() returns true. And since shortcode handles in our plugin are registered only when is_admin() returns false, it leads to inability to display our shortcodes content on frontend.

I don't want to load unnecessary files when those aren't needed for performance-related reasons, so some kind of workaround is needed for this.

I have two ideas for this:

a. Make a "whitelist" of such page builders (I've got few names like Thrive Ultimate) and load shortcode-related files and stuff when plugin detects one of those.

or

b. Add "Compatability with page builders" settings option which, when enabled, makes plugin load all shortcodes-realted stuff.

Will do some tests to see which one is preferable. I'm for option b though as we wouldn't have to manually whitelist page builders in this case.

Arsenal21 commented 5 years ago

Option b sounds good to me too.

erommel commented 5 years ago

I did some research on this. I figured there is almost no performance impact to load shortcode-related functionality even on admin site. I run some tests on my old home server (which is old Atom N450 based notebook https://ark.intel.com/content/www/us/en/ark/products/42503/intel-atom-processor-n450-512k-cache-1-66-ghz.html) and there is no difference performance-wise.

So the only reason to add this option now is to resolve another page builder conflict when those won't let standard WP routines run and this prevents plugin from outputting JavaScript vars.

erommel commented 5 years ago

Main issue with page builders is that some of them prevent wp_localize_script() and wp_enqueue_script() functions from working properly. Here's what can be done to get over it:

  1. Add scripts and styles on every page regardless if there are payment buttons on them or not. This would add a couple of additional HTTP requests to every page of the site though, but if users are using some CSS\JS combining plugins, this shouldn't be a problem.

  2. Echo required JavaScript vars instead of using wp_localize_script(). Actually the function does the same thing - it just echoes vars. We'll just do it by ourselves and no page builder can stop us from doing this :) Example here: https://github.com/Arsenal21/paypal-for-digital-goods/blob/master/paypal-for-digital-goods/public/includes/class-shortcode-ppdg.php#L178

  3. We will also echo a small portion of JavaScript that would run required routines only if they're needed (e.g. only if there are payment buttons on page). This would be great performance-wise. Example here: https://github.com/Arsenal21/paypal-for-digital-goods/blob/master/paypal-for-digital-goods/public/includes/class-shortcode-ppdg.php#L261

  4. Rewrite our public JavaScript to not do anything unless specifically instructed. Currently it searches for active buttons on page load and attaches required routines for them. Instead, buttons should echo a small JavaScript portion to instruct the script to do the routines. This is related to performance and to number 3 from this list. Example here: https://github.com/Arsenal21/paypal-for-digital-goods/blob/master/paypal-for-digital-goods/public/assets/js/public.js

  5. Also make sure addons that are using frontend scripts do the same thing.

This way we should beat most (if not all) page builders\visual composers\custom theme issues at the cost of couple additional HTTP requests (which can be dealt with using CSS\JS combining plugins).

erommel commented 5 years ago

Last, but not least - I think this should be done for NG only (https://github.com/Arsenal21/stripe-payments/tree/ng) as there is little sense in redoing deprecated Stripe's Checkout which is currently used.

Arsenal21 commented 5 years ago

okay cool. Yeah a few extra requests completely fine for this. And yeah lets only do this for the "../tree/ng".

erommel commented 5 years ago

This one can be closed as well. Settings option is not required as plugin now tries it's best to be compatible with page builders.