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

[BUG] Timber::get_pages_menu() throws warnings when using Polylang #2922

Open
lnorby opened this issue Feb 15, 2024 · 6 comments
Open

[BUG] Timber::get_pages_menu() throws warnings when using Polylang #2922

lnorby opened this issue Feb 15, 2024 · 6 comments
Labels

Comments

@lnorby
Copy link

lnorby commented Feb 15, 2024

Expected Behavior

Expected to not get any warning messages.

Actual behavior

Warning:
Undefined array key "menu" in
wp-content\mu-plugins\polylang\frontend\frontend-nav-menu.php
on line 280

Warning:
Undefined array key "theme_location" in
wp-content\mu-plugins\polylang\frontend\frontend-nav-menu.php
on line 294

Steps to reproduce behavior

No response

Notes

No response

What version of Timber are you using?

2.0

What version of WordPress are you using?

6.4.3

What version of PHP are you using?

8.1

How did you install Timber?

Installed or updated Timber through Composer

@Levdbas
Copy link
Member

Levdbas commented Feb 16, 2024

Hi @lnorby , This might have something to do with how you are setting up your menus.

if you are using the Timber::get_menu function you could try this:

// use 
$menu = Timber::get_menu( 'my-menu', [
    'menu' => YOUR_MENU_ID,
    'theme_location' => 'YOUR_REGISTERED_MENU_LOCATION'
  ]
 );

If that does not help, please post the code how you set up the menu that causes this issue.

Edit: @timber/rangers

This might be a bug?

Here we set $args['location'] . Shouldn't that be mimicking wp_nav_menu_args so$args['location'] becomes $args['theme_location']

@lnorby
Copy link
Author

lnorby commented Feb 16, 2024

I am using Timber::get_pages_menu(). I tried the following, and it worked. No warnings now.

Timber::get_pages_menu( [
    'menu' => '',
    'theme_location' => '',
] )

Maybe you could set these as default arguments for the method.

Edit: Timber::get_menu() has no warnings with Polylang, only Timber::get_pages_menu().

@Levdbas
Copy link
Member

Levdbas commented Feb 16, 2024

Hmm, looking at the default arguments for wp_page_menu()'s which Timber is using, there are no menu and theme_location arguments. https://developer.wordpress.org/reference/functions/wp_page_menu/

So adding these two parameters to get_pages_menu is not something that is standard in WordPress as well. Polylang might be checking for the wrong parameters when handling a pages menu?

@lnorby
Copy link
Author

lnorby commented Feb 16, 2024

The wp_page_menu() function itself is working fine with Polylang, no warnings.

@gchtr gchtr added the 2.0 label Feb 16, 2024
@Levdbas
Copy link
Member

Levdbas commented Feb 16, 2024

Hi @lnorby ,

I took another look at this and I think what leads to your error is the following:

  1. [the pages menu by wordpress takes a wp_page_menu and transforms it to an actual menu
  2. to be compatible with both the wordpress pages function and the wordpress menu we apply both the wp_page_menu_args and wp_nav_menu_args filters in Timber::get_page_menu()
  3. Since Polylang is adding a filter here to wp_nav_menu_args that assumes menu and 'theme_location' are always set, this fails. I think it is right for them to assume those properties are always set since they are by default set by WordPress. and thus the error occurs.

@timber/rangers , is this something the encourage to fix by the user themself or do we want to improve the compatibility since we are actively calling the wp_nav_menu_args without providing all the arguments?

@gchtr
Copy link
Member

gchtr commented Feb 26, 2024

@timber/rangers , is this something the encourage to fix by the user themself or do we want to improve the compatibility since we are actively calling the wp_nav_menu_args without providing all the arguments?

@timber/rangers We’ve already done a lot of work to make Timber more compatible with native menu filters. If we can improve compatibility here, I think we should.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants