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

[New sniff] No hard-coded menu names in nav_menu functions #93

Open
3 tasks
jrfnl opened this issue Dec 18, 2016 · 3 comments
Open
3 tasks

[New sniff] No hard-coded menu names in nav_menu functions #93

jrfnl opened this issue Dec 18, 2016 · 3 comments

Comments

@jrfnl
Copy link

jrfnl commented Dec 18, 2016

Rule type:

Error / Warning

Rule:

nav-menu check

In the (register|wp)_nav_menu() function, the theme should only use a variable for the menu name. No hard-coded menu names allowed.

The rule should be clarified and added to the handbook before any action is taken on this issue.

Ref: https://make.wordpress.org/themes/handbook/review/required/theme-check-plugin/#info

Theme check file covering this rule:

Unknown
Loosely related: WordPress/theme-check#142

Decision needed:

  • Is this really a rule ?
  • If so, should it be a warning or error ?

To do:

  • Add the rule in the Theme Review handbook
  • Create unit tests
  • Create new sniff
@grappler
Copy link
Member

This falls under

Avoid hard coding to modify content. Instead, use function parameters, filters and action hooks where appropriate.

This should be an error.

By using a hardcoded a hard coded menu name it would require the menu to have the exactly same name which does not make sense for a publically released theme.

@joyously
Copy link

I think you are mistaken. A theme names its menu locations, not the user. They should not have to be in variables. The menu location is not "content" as mentioned in that rule you quoted.

@grappler
Copy link
Member

A theme names its menu locations, not the user.

You are right but we are not talking about the location but the menu name.

These are the two parameter which is causing the confusing. We want to restrict the first not the second.

  • 'menu' (int|string|WP_Term) Desired menu. Accepts (matching in order) id, slug, name, menu object.
  • 'theme_location' (string) Theme location to be used. Must be registered with register_nav_menu() in order to be selectable by the user.

I hope this makes sense.

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

No branches or pull requests

4 participants