cuny-academic-commons / bp-event-organiser

Allows Event Organiser plugin events to be assigned to BuddyPress groups and generates a group calendar page for each group
GNU General Public License v2.0
3 stars 1 forks source link

Select2 library conflict #48

Closed danaskallman closed 8 years ago

danaskallman commented 8 years ago

Found that there is a conflict with woocommerce, the site is also using CiviCRM.

From what we could find this pluing is loading a newer version of select2

wp-content/plugins/bp-event-organiser/includes/functions.php: wp_register_script( 'select2', set_url_scheme( 'http://cdnjs.cloudflare.com/ajax/libs/select2/4.0.0-rc.2/js/select2.min.js', array( 'jquery' ) ) );

wp-content/plugins/bp-event-organiser/includes/functions.php: wp_register_style( 'select2', set_url_scheme( 'http://cdnjs.cloudflare.com/ajax/libs/select2/4.0.0-rc.2/css/select2.min.cs…' ) );

WooCommerce is using 3.5.4 and CiviCRM is using 3.5.2

Thoughts on how to resolve conflict?

r-a-y commented 8 years ago

We use Select2 v4.0.2. See commit ca4a1b5.

I would recommend that the other plugins upgrade their JS to use the newer version of Select2.

You could also try de-registering BPEO's Select2 library on WooCommerce / CiviCRM pages. Something like:

function my_bpeo_deregister_assets() {
     // Replace "CHECK_IF_WOO_OR_CIVICRM_PAGE" with actual conditional functions.
     if ( CHECK_IF_WOO_OR_CIVICRM_PAGE ) {
          wp_deregister_script( 'select2' );
     }
}
add_action( 'wp_enqueue_scripts',    'my_bpeo_deregister_assets', 6 );
add_action( 'admin_enqueue_scripts', 'my_bpeo_deregister_assets', 6 );
boonebgorges commented 8 years ago

Yeah, unfortunately, I think that the suggestion by @r-a-y is as good as we're going to get. Select2 broke a fair amount of backward compatibility between 3.x and 4.x, so there's no way to reconcile the client code between the two. And while it is theoretically possible to namespace one or other of the select2 instances, it should be on those who need to use a legacy version, not us ;)

In the meantime, I definitely recommend something like what @r-a-y has suggested. Sorry for the trouble!

On 04/05/16 13:25, r-a-y wrote:

We use Select2 v4.0.2. See commit ca4a1b5 https://github.com/cuny-academic-commons/bp-event-organiser/commit/ca4a1b58eb8627cff7ec547d4b3e87acfa0f90f9.

I would recommend that the other plugins upgrade their JS to use the newer version of Select2.

You could also try de-registering BPEO's Select2 library on WooCommerce / CiviCRM pages. Something like:

|function my_bpeo_deregister_assets() { // Replace "CHECK_IF_WOO_OR_CIVICRM_PAGE" with actual conditional functions. if ( CHECK_IF_WOO_OR_CIVICRM_PAGE ) { wp_deregister_script( 'select2' ); } } add_action( 'wp_enqueue_scripts', 'my_bpeo_deregister_assets', 6 ); add_action( 'admin_enqueue_scripts', 'my_bpeo_deregister_assets', 6 ); |

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/cuny-academic-commons/bp-event-organiser/issues/48#issuecomment-205931280

christianwach commented 8 years ago

@danaskallman Where are you seeing the conflict? On the front-end or in wp-admin?

If it's wp-admin you're talking about, I coded the CiviCRM-WordPress plugin such that CiviCRM only loads its assets on the CiviCRM admin pages themselves. This should rule out conflicts with Civi on the back-end, but if not, please let me know and I'll take a look. On the front-end, I can imagine a situation where there's Civi content (via a shortcode) and BPEO content on the same page (e.g. an archive page of some sort), but I'd be surprised in this plugin actually loads Select2 in those situations. Again, please correct me if I'm wrong.

It does indeed appear that WooCommerce registers its version globally in wp-admin which is perhaps... over-enthusiastic. I see that Select2 4.x is the subject of many issues, e.g. this one, but their roadmap for 2.5 skipped over it. The technique provided by @r-a-y is what other plugins that need compatibility with WooCommerce seem to use. EDIT: it seems to be the other way round - plugins deregister Woo's version of Select2 and register their own when needed. I agree with @boonebgorges that you are best placed to control the script registration process for your particular project.

kcristiano commented 8 years ago

@christianwach Understand about the CiviCRM loading, the issue we saw was on the Woo Product admin page the bp-event-organiser plugin's version of select2 was loading. I expected that it would not have targeted all wp-admin pages, to be fair I did not look at the code, but suspect I'll do that now. EDIT - I do see that you check for a previously registered version of select2 and if none exists you load yours. I see the issue is the load order.

Thanks for the feedback.

christianwach commented 8 years ago

the issue we saw was on the Woo Product admin page the bp-event-organiser plugin's version of select2 was loading. I expected that it would not have targeted all wp-admin pages

Both plugins do indeed register their Select2 script globally, but should only load them in appropriate places. The confusion seems to lie in which version is registered first.

@r-a-y which wp-admin pages does BPEO use Select2 on? Perhaps this plugin can be tweaked to restrict script registration to just the pages where it's needed?

boonebgorges commented 8 years ago

The confusion seems to lie in which version is registered first.

Yeah. This is pretty lame. In general, registering your assets early and then enqueuing them just-in-time is best practice. That's because all plugins and themes should be able to share the assets. But this is a case where that strategy won't work. It's not ideal, but I'll go ahead and change our script loading technique so that scripts are only registered when used. This should mostly limit the conflicts (since IIRC we don't use Select2 on wp-admin at all).

christianwach commented 8 years ago

I'll go ahead and change our script loading technique so that scripts are only registered when used. This should mostly limit the conflicts (since IIRC we don't use Select2 on wp-admin at all).

@boonebgorges Awesome. Thank you!

kcristiano commented 8 years ago

@boonebgorges Thank you, This part has always been lame. Really appreciate this.

boonebgorges commented 8 years ago

@kcristiano @danaskallman I think that b296c4d should fix the specific problem - it changes BPEO so that our scripts (including Select2) are only registered on the few pages where we need them (our front-end event/calendar pages, and edit.php?post_type=event). This should fix any problems where WooCommerce was unable to run because of BPEO. It will probably not fix the case where WooCommerce's version of Select2 is registered first (which it now always will be), so something like what @r-a-y suggests above may still be necessary to make sure that bp-event-organiser works as expected. I encourage you to give it a try and let me know. Thanks!