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.

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

Add compatibility with WooCommerce Multilingual plugin #1530

Merged
merged 1 commit into from
Sep 23, 2015

Conversation

mikaelmattsson
Copy link
Contributor

I experienced a conflict between Sage and a WPML and WooCommerce combination, where a translated product archive (specificly product_cat taxonomy page) wouldn't be wrapped in the base template. Just the template file contents would be executed without the master/base template. This could be solved by changeing the priority of the template_include filter for the wrapper to >100. I'm not sure if this is WooCommerce specific or if this would be the case for any translated taxonomy when using WPML.

I experienced a conflict between Sage and a WPML and WooCommerce combination, where a translated product archive (specificly product_cat taxonomy page) wouldn't be wrapped in the base template. Just the template file contents would be executed without the master/base template. This could be solved by changeing the priority of the template_include filter for the wrapper to >100. I'm not sure if this is WooCommerce specific or if this would be the case for any translated taxonomy when using WPML.
@QWp6t
Copy link
Member

QWp6t commented Aug 13, 2015

It doesn't look like WPML or WooCommerce set a priority above 99.

WooCommerce hooks into template_include in includes/class-wc-template-loader.php#L17

WPML hooks into template_include in inc\template-functions.php#L967 and inc\url-handling\wpml-root-page.class.php#L24.

We also have members of the Roots team who have used both plugins together with Sage and have not experienced any issues. I'll leave this open for the time being in case there's something you think I'm missing, but at this point, I don't think this is a Sage issue.

@mikaelmattsson
Copy link
Contributor Author

Yep, you are right. I checked. It's not in WooCommerce or the main WPML plugin but in the WPML addon plugin, WooCommerce Multilingual.
In woocommerce-multilingual/inc/store-pages.class.php#L14 there is the following code

add_filter( 'template_include', array( $this, 'template_loader' ), 100 );

Looking at the contents of that method I'm pretty sure thats whats causing the problem. :)

@mikaelmattsson
Copy link
Contributor Author

@QWp6t ?

@Foxaii
Copy link
Contributor

Foxaii commented Aug 17, 2015

You're correct about the cause of the issue and the fix.

The problem with changing filter priorities is that you can easily break other plugins/code that need a higher priority to work.

@mikaelmattsson
Copy link
Contributor Author

True. No one knows until you try. It might cause incompatibility with some other plugin, and it might even fix compatibility with another plugin. All we know is that the SageWrapping is a very important filter and should probably allways have the highest priority (executed last).

Increase the priority to 101, to be a bit more cautious and then pray to the dev gods?

@QWp6t
Copy link
Member

QWp6t commented Aug 17, 2015

I think we're okay with raising the priority. We're just discussing what number to settle on. We've discussed 1000, 999, etc. I also suggested PHP_INT_MAX or PHP_INT_MAX - 1, but I think the consensus is that's too high.

@retlehs retlehs changed the title Add compatibility with WPML and WooCommerce Add compatibility with WooCommerce Multilingual plugin Aug 20, 2015
retlehs added a commit that referenced this pull request Sep 23, 2015
Add compatibility with WooCommerce Multilingual plugin
@retlehs retlehs merged commit dce3c27 into roots:master Sep 23, 2015
@mikaelmattsson mikaelmattsson deleted the patch-1 branch September 23, 2015 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants