-
Notifications
You must be signed in to change notification settings - Fork 38
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 disabling of the admin toolbar. #1
Comments
By the looks of it, there already is a sniff for this in WP-VIP: https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/develop/WordPress/Sniffs/VIP/AdminBarRemovalSniff.php The check in Theme Check however is more comprehensive, so I suggest improving the VIP check upstream and then adding it to the Theme rule set. |
@jrfnl Was there something specific you saw that needed improving other then add a check for |
@grappler I believe those checks should be done on both the PHP as well as CSS files and should be quite specific. I mean, a theme changing the color of the admin bar to be in line with their front-end theme should not see this notice. I also think that for the existing PHP function based check, the parameter values should be checked against some whitelisted values. I also think the sniff should be moved from I have made a small start locally for the changes needed upstream - mainly creating the unit tests. Want me to share this for you to continue ? |
Definitely agree. I've said as much and thought that we had decided not to target CSS a while back. I think we'd all be OK with a check for something like this (or variations):
Force-showing is as equally bad as force hiding it. Whether to show/hide the admin bar is outside the scope of themes. This is core and plugin territory. |
The altered sniff will need to be as re-usable as possible as it is used for plugins as well in VIP context. So in that case, we either need a property switch which allows for checking that those functions are used at all (themes) or checking that those functions don't hide the admin bar (rest of the world). |
Rule:
No hiding of the admin bar - check if
show_admin_bar( false )
is called or ifadd_filter( 'show_admin_bar', '__return_false' )
is somewhere in the code.Ref: https://make.wordpress.org/themes/handbook/review/required/#core-functionality-and-features
Theme check file covering this rule:
https://github.com/Otto42/theme-check/blob/master/checks/adminbar.php
To do:
- [ ] Create unit tests- [ ] Create new sniffWordPress_Sniffs_VIP_AdminBarRemovalSniff
sniff to the ruleset.The text was updated successfully, but these errors were encountered: