-
Notifications
You must be signed in to change notification settings - Fork 130
Fix incorrect usage of badge colors in all Site Health checks #472
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -40,15 +40,15 @@ function perflab_aao_autoloaded_options_test() { | |||
$base_description = __( 'Autoloaded options are configuration settings for plugins and themes that are automatically loaded with every page load in WordPress. Having too many autoloaded options can slow down your site.', 'performance-lab' ); | |||
|
|||
$result = array( | |||
'label' => esc_html__( 'Autoloaded options are acceptable', 'performance-lab' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed we are applying esc_html__
inconsistently to translations and that you removed a bunch in this PR.
For core, we don't escape translations, however for plugins I generally recommend escaping (as 10up recommends).
Maybe rather than changing here we can open a follow issue to use a uniform approach and change throughout the plugin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While escaping is recommended for plugins more than core, the rules also say that escaping should happen at the output level. In all the places here though, we're only writing data into an array, so per those guidelines escaping here would not follow the best practice.
All escaping for Site Health check content should happen in WordPress core per that definition, since that's where the data is being output.
Summary
Fixes #471
Relevant technical choices
blue
.Checklist
[Focus]
orInfrastructure
label.[Type]
label.no milestone
label.