#46734 closed enhancement (fixed)
Site Health: Add actions to all site status tests
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.2 | Priority: | normal |
Severity: | normal | Version: | 5.2 |
Component: | Site Health | Keywords: | site-health has-patch commit |
Focuses: | ui, docs | Cc: |
Description
Hello,
It would be nice to include links to the themes/plugins/update screens where applicable such as the 'Inactive plugins should be removed' and 'You should remove inactive themes' improvements.
Thank you
Attachments (14)
Change History (36)
#1
@
6 years ago
Not sure if it would need to open in a new tab, the external icon mainly got in there as I copied the button from the PHP update recommendation.
#3
@
6 years ago
- Keywords site-health added
- Milestone changed from Awaiting Review to 5.2
- Summary changed from Site Health: Add buttons for 'Manage plugins' and 'Manage themes' to the inactive improvements to Site Health: Add actions to all site status tests
This is something we were intending to do, all tests should have actions linking off to either documentation, or pages to resolve the reported problems.
Quick remark that we should ensure they are links as long as they direct away from the current page you are on, as recommended by the accessibility team.
Related #46573
#4
follow-up:
↓ 5
@
6 years ago
46734.patch starts to introduce actions for various tests.
There are a few missing, and I'm not quite sure what resources they could link to that would make sense. Some have helpful texts, directing the user to contacting their web host (for example to update PHP, update SQL, or missing PHP libraries), while others are a bit more vague as to what resources should be used, for example:
- Your site can perform loopback requests
- Your site can communicate securely with other services
- Scheduled events are running (or failing)
These are just a few of the ones missing actions, there are more, especially if you consider potential failure states as well.
I know what these mean, the short description following each will hopefully help users understand as well, but these are all things that the average user may find hard to troubleshoot, and in many cases end up being "Contact your host", which we of course have no direct way of linking to.
So I guess the overarching question is, should we just omit actions in these cases, or should we (like I did for one of them) link to the WordPress support forums with a "get help" kind of text?
#5
in reply to:
↑ 4
@
6 years ago
- Keywords has-patch needs-refresh added
Replying to Clorith:
46734.patch starts to introduce actions for various tests.
Thanks for initial patch there @Clorith
There are a few missing, and I'm not quite sure what resources they could link to that would make sense. Some have helpful texts, directing the user to contacting their web host (for example to update PHP, update SQL, or missing PHP libraries), while others are a bit more vague as to what resources should be used, for example:
- Your site can perform loopback requests
- Your site can communicate securely with other services
- Scheduled events are running (or failing)
These are just a few of the ones missing actions, there are more, especially if you consider potential failure states as well.
I know what these mean, the short description following each will hopefully help users understand as well, but these are all things that the average user may find hard to troubleshoot, and in many cases end up being "Contact your host", which we of course have no direct way of linking to.
So I guess the overarching question is, should we just omit actions in these cases, or should we (like I did for one of them) link to the WordPress support forums with a "get help" kind of text?
For these more superfluous tests if there's nothing to link to then I feel it's fine to leave especially if it's a 'contact your host' as the call to action is in the verbiage and don't have a link we can determine.
However, if there is further information that could be provided potentially as a HelpHub page that could be very useful to the user and the interface as it could help reduce the content in the accordion while providing more information and potential troubleshooting steps via an external article.
Reviewing the patch some minor thoughts;
- 'please contact your host' can this be 'please contact your host or developer' as in many cases the person who would deal with the change is their developer or agency and not the host themselves.
- Should the URL 'https://wordpress.org/support' be translatable?
- Can the string 'Remove inactive plugins' be updated to 'Manage inactive plugins' to avoid hesitation. Reading 'Remove inactive plugins' I didn't want to click it thinking it may conduct that action for me when I would prefer to review the list and then remove which is what is happening.
Thanks for the amazing work @Clorith stoked to see Site Health making it's way to core.
#6
@
6 years ago
- Focuses docs added
@Clorith thanks for your patch and @garrett-eclipse thanks for review that patch.
Reviewing the patch and found some minor changes that need to fix also.
- When use sprintf function with multiple %s then it is better to use %1$s and %2$s respectively
- Use a consistent wording 'please contact your host' should be 'Please contact your server administrator.'
@garrett-eclipse as per core software URL 'https://wordpress.org/support' is not translatable.
Manage inactive plugins text is good option.
#8
@
6 years ago
- Keywords needs-testing added
Thanks for the feedback @mukesh27 and patch @vaishalipanchal. I uploaded 46734.2.diff to replace the comma preceding the new 'Please...' with a period as well as updated one more instance of your host with your server administrator, I do like that term it's more encompassing.
Cheers
#9
follow-up:
↓ 10
@
6 years ago
The reason you want to use %1$s
and similar is if there are multiple attributes within a translation string that are better suites in a different order. Here the order is fixed, the first element (the link) always has to go inside the a
tag and the second one, the string, is always the second one. Adding ordered replacements here add a potential failure element if used incorrectly so as the coding standards don't state that they must be used I'd prefer the use of the fixed format when possible, just to be on the safe side.
@garrett-eclipse I agree with you on the wording, it sounds less frightening that way to a regular user without technical experience.
We should also make the URL to the support area translatable as suggested.
#10
in reply to:
↑ 9
@
6 years ago
Thanks @Clorith I appreciate the review/feedback.
Replying to Clorith:
The reason you want to use
%1$s
and similar is if there are multiple attributes within a translation string that are better suites in a different order. Here the order is fixed, the first element (the link) always has to go inside thea
tag and the second one, the string, is always the second one. Adding ordered replacements here add a potential failure element if used incorrectly so as the coding standards don't state that they must be used I'd prefer the use of the fixed format when possible, just to be on the safe side.
This makes sense for sure, might be nice to get that into the coding standards, within my updated patch I've set all static strings to use %s
and left the translatable strings using the %1$s
convention.
We should also make the URL to the support area translatable as suggested.
I've updated the support URL to make it translatable and searched core to find all references I could and they were all translatable.
And one final addition with the actions being fairly consistent I updated the 'Read more about why you should use HTTPS' link to use the convention making it an external link w/ the icon.
Would love another review and testing, uploaded screens to illustrate changes.
Other notes;
- There's no filter for inactive themes hence why it doesn't have the same second action link that plugins has.
- The 'One or more recommended modules are missing' section has a link to the 'the team handbook', should this be an action link?
- Is there a handbook URL for 'Background updates are not working as expected'?
- Which WP.org links should be translatable? Some like support make sense but others don't have variants so could be removed from the load on translators. Below is recon on this for
class-wp-site-health.php
;
Translatable;
https://make.wordpress.org/hosting/handbook/handbook/server-environment/#php-extensions
https://wordpress.org/about/requirements/
https://wordpress.org/support
https://wordpress.org/support/article/why-should-i-use-https/
Not Translatable;
https://wordpress.org/support/article/debugging-in-wordpress/
#11
@
6 years ago
They should all be translatable, HelpHub has stuff in the works and once localized versions start coming out we can then easily make them available without needing core updates.
We don't have a handbook page for the background updates not working, there's a "something in your setup is wrong" scenario, there's multiple potential failures, so telling them to reach out to their server admin is the best approach there.
For the team handbook link, that's intended to be a part of the description (it's just letting you know where the reason and data is sourced from), so it shouldn't be an action link (since it's not an actionable area, it's merely contextual).
I approve of the inclusion of an external indicator, we should probably check with design on those since the icons are huge and really break the link string in my opinion (just removing the underline would do the trick I suspect, but I don't want to mess with existing conventions without them seeing that it's OK to do so).
This ticket was mentioned in Slack in #design by clorith. View the logs.
6 years ago
#13
@
6 years ago
After a quick discussion in the design channel, let's remove the link-underlines from the icons here.
@
6 years ago
Translated all help links, included translator comments for links, updated external icon via css to remove underline
#14
@
6 years ago
Thanks @Clorith I appreciate the feedback and for raising that with design.
In 46734.4.diff, I've translated the last URL as well as provided translator comments for all of the existing links. And added css to remove the underline on the external icon.
Please review,
Cheers
#15
@
6 years ago
- Keywords has-patch needs-testing removed
46734.4.diff looks good code-wise, but the string changes to "Manage inactive plugins" and such are undone, once those are fixed this should be good to go.
#16
@
6 years ago
- Keywords needs-refresh added
As per WPCS https://make.wordpress.org/core/handbook/best-practices/coding-standards/css/#selectors Refrain from using over-qualified selectors, div.container
can simply be stated as .container
@garrett-eclipse Can you please replace below css or add unique css in span.
.health-check-accordion-panel a span.dashicons-external { text-decoration: none; }
Replace to
.health-check-accordion-panel a .dashicons-external { text-decoration: none; }
@
6 years ago
Updated patch to restore the 'Manage inactive plugins' string and be less specific with the css, also with the CSS selected went with the generic .dashicons over .dashicons-external to future proof.
#17
@
6 years ago
- Keywords has-patch needs-testing added; needs-refresh removed
Thanks for catching that string reversion @Clorith I've caught that but did a scan and feel that's the only one amiss, can you recheck please.
And thanks @mukesh27 for catching that CS issue, I'm learning those rules more each day ;)
The updated patch makes the selector less specific as well I updated to use just .dashicons and not .dashicons-external to future proof in case other dashicons are used in any of the links.
Can you guys please retest/review and we'll get this in before the string freeze (hopefully).
Cheers
#18
@
6 years ago
- Keywords commit added
Let's roll with this, there's a WPCS error, but it can be automatically fixed, and doesn't relate to strings, so let's get this in.
#19
@
6 years ago
- Keywords needs-testing removed
Thanks @Clorith for flagging that, I used the issue to learn how to setup wpcs/phpcbf so was able to identify and remove the extra newline there. Uploaded 46734.6.diff to address the WPCS error.
#20
@
6 years ago
46734.6.diff looks good @garrett-eclipse, unless there is anything else, I say we ship this
Current improvements for inactive plugins/themes