Show check results on admin page

[jjgrainger]

Addresses acceptance criteria in #35

Closes #35

[spacedmonkey]

@jjgrainger There is no styling on the table. Is this expected?

[spacedmonkey]

When I click on a plugin without an error or warning, I see no message at all. It seems like the plugin checker has done nothing here. This maybe hard to test, if plugin even works.

[jjgrainger]

Thanks @spacedmonkey

I've updated the PR based on your feedback. As for your comment...

When I click on a plugin without an error or warning, I see no message at all. It seems like the plugin checker has done nothing here. This maybe hard to test, if plugin even works.

I agree, and for now I've added a simple 'Checks complete' message once the process is done. I think it would be good to discuss what output through the process we'd want to display as part of the milestone review. Can you create an issue for this one?

[spacedmonkey]

This is looking a lot better!

Couple of things. If I run this a couple of times, I see complete message twice, I don't know why this is.

Another nitpic, but would not take long to implement, when you click the button, if you a spinner could show, then go away when request is complete that would be amazing. Jetpack took a while to process and did not know when it was complete. To add a spinner in WP is very simple.

Next to the submit button add this.

<span class="spinner"></span>

It will look like this.

Add a html id to it. When the button is clicked, add is-active class to the id and remove when the requests are complete. Should not take more than a couple of minutes to add and will make it clear to the user that something is happening.

[spacedmonkey]

I think barring the spinner, this is nearly there.

[felixarntz]

@jjgrainger Left some feedback here and above, but most of it looks good so far.

[felixarntz]

A few things here:

• These files should preferably only include the templates themselves, not the surrounding script tag. For that we should use the wp_print_inline_script_tag() function. To pass the output of these templates into the function, we can then use an output buffer.
• No need for require_once, this file doesn't contain PHP functions or anything like that and therefore is okay to be included multiple times.

Suggested change

	require_once WP_PLUGIN_CHECK_PLUGIN_DIR_PATH . '/templates/results-table.php';
	require_once WP_PLUGIN_CHECK_PLUGIN_DIR_PATH . '/templates/results-row.php';
	ob_start();
	require WP_PLUGIN_CHECK_PLUGIN_DIR_PATH . '/templates/results-table.php';
	$results_table_template = ob_get_clean();
	wp_print_inline_script_tag(
	$results_table_template,
	array(
	'id' => 'tmpl-plugin-check-results-table',
	'type' => 'text/template',
	)
	);
	ob_start();
	require WP_PLUGIN_CHECK_PLUGIN_DIR_PATH . '/templates/results-row.php';
	$results_row_template = ob_get_clean();
	wp_print_inline_script_tag(
	$results_row_template,
	array(
	'id' => 'tmpl-plugin-check-results-row',
	'type' => 'text/template',
	)
	);

[felixarntz]

@jjgrainger One last comment here, but this is pretty much good to go.

[joemcgill]

This is looking good to me. I may have found a small bug that we could improve. It looks like if you click the button more than once wild checks are running, you will end up with multiple instances of the results. Probably the best way to handle this is to disable the button while the spinner is active.

[felixarntz]

@jjgrainger Thank you for the updates. Approved, pending the feedback from @joemcgill, it would be great to make the button temporarily disabled until the process is complete (or has terminated due to an error).

[joemcgill]

The latest commit fixes the double results issue for me. Thanks, @jjgrainger!

Jonny is OOO, and his feedback has been addressed.