[wp-trac] [WordPress Trac] #46573: Introduce a Site Health module for users to self-service their site

[WordPress Trac]

#46573: Introduce a Site Health module for users to self-service their site
-----------------------------------------------------+---------------------
 Reporter:  Clorith                                  |       Owner:  (none)
     Type:  enhancement                              |      Status:  new
 Priority:  normal                                   |   Milestone:  5.2
Component:  General                                  |     Version:
 Severity:  normal                                   |  Resolution:
 Keywords:  has-patch has-screenshots needs-refresh  |     Focuses:
-----------------------------------------------------+---------------------
Changes (by pento):

 * keywords:  has-patch has-screenshots => has-patch has-screenshots needs-
     refresh


Comment:

 Thank you for wrangling this patch, @Clorith! My apologies for reviewing
 it so late in the process. I think we're generally heading in the right
 direction, but there is some work to be done still. Some of it should
 probably be done before beta 1, but a lot of it can be put off until after
 then.

 Generally speaking, the biggest issue I ran into was the lack of comments.
 Both docblocks and inline comments explaining some of the trickier code
 would've helped a lot.

 On the JavaScript:
 - I noticed quite a few coding standards issues. Linting it should help.
 - Is there a particular reason why things are split into multiple
 `jQuery.ready()` calls?
 - Because everything is scoped inside `jQuery.ready()` calls, here's no
 need for the `HC` prefix in function names.
 - I'm not wild about the HTML template in `HCAppendIssue()` being
 generated as a string. Could this be done in a more reusable manner?
 - It feels like a lot of the error condition checking in
 `HCRecalculateProgression()` is redundant, as the calculations should be
 taking care of weird values.

 For the CSS:
 - A lot of these rules seem like there should be equivalent versions
 already in Core, they're fairly generic.
 - Several of the colours don't match those in the
 [https://make.wordpress.org/design/handbook/design-
 guide/foundations/colors/ Design Handbook]. Could you double check them
 all?
 - I seem to recall `flex` layouts being a bit janky in IE11. Could you
 test that it works correctly?

 Into the PHP:
 - I'm a bit nervous about the magic Ajax calls which correspond to
 particular method names. This seems like it could be better done using
 actions. As we're exposing a lot of sensitive information through these
 calls, the less magic involved, the better.
 - We don't need to check `defined( 'ABSPATH' )` at the top of each file.
 - All of the `esc_html__()` and `esc_html_e()` calls should be replaced
 with `__()` and `_e()`, respectively. We perform appropriate safety checks
 on strings in GlotPress, instead.
 - The PHP and MySQL versions in `WP_Site_Health` need to be bumped to 5.6
 and 5.5, respectively.

 This is just written from a code read-through, I skipped auditing each of
 the tests, but that would be a good thing to spend time on.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/46573#comment:14>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform