Gist Shortcode: Remove jQuery Dependency

[Aurorum]

Part of #16409

Changes proposed in this Pull Request:

• Removes the use of jQuery for the Gist shortcode

Jetpack product discussion

• N/A

Does this pull request change what data or activity we track or use?

Nope.

Testing instructions:

Try using a Gist shortcode with different ts parameters. For example:

[gist jeherve/57cc50246aab776e110060926a2face2] (ts defaults to 8)
[gist ts="2" jeherve/57cc50246aab776e110060926a2face2]

Verify that the Gist appears with the additional styling, and the tab-size style is also inserted.

Proposed changelog entry for your changes:

• Shortcodes: remove jQuery dependancy from Gist shortcode.

[jetpackbot]

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16700

Scheduled Jetpack release: September 1, 2020.
Scheduled code freeze: August 25, 2020

Generated by 🚫 dangerJS against 6314a9f

[jeherve]

Tests appear to be failing right now. Could you take a look?
https://github.com/Automattic/jetpack/pull/16700/checks?check_run_id=944382555

[Aurorum]

I think that I've fixed the tests now! :)

[jeherve]

Thank you. I'm a bit worried of us removing a CSS class that may be used by some to customize the look of the embed on their site. Maybe we should keep it around?

[Aurorum]

The gist-oembed class? That appears to only be used by the jQuery to replace the empty div with the actual Gist. It's replaced as soon as it loads, so there shouldn't be anyone using it for styling purposes!

[jeherve]

It may be worth testing to see if this also happens to fix #10161. 🤞

[Aurorum]

I must admit that I'm not sure that I fully understand that issue, but it appears to be triggering a console error still with no visible fixes. It's possible that I've misunderstood it though.

[jeherve]

This tests well for me. I would have a few suggestions though, see below.

Here is a diff incorporating the different changes I suggested. Let me know what you think about it!

diff --git a/modules/shortcodes/gist.php b/modules/shortcodes/gist.php
index ad51b5119..c8403257c 100644
--- a/modules/shortcodes/gist.php
+++ b/modules/shortcodes/gist.php
@@ -201,22 +201,29 @@ function github_gist_shortcode( $atts, $content = '' ) {
 	}
 
 	// URL points to the entire gist, including the file name if there was one.
-	$id = ( ! empty( $file ) ? $id . '?file=' . $file : $id );
+	$id     = ( ! empty( $file ) ? $id . '?file=' . $file : $id );
 	$return = false;
 
-	$request      = wp_remote_get( "https://gist.github.com/" . esc_attr( $id ) );
+	$request      = wp_remote_get( esc_url_raw( 'https://gist.github.com/' . esc_attr( $id ) ) );
 	$request_code = wp_remote_retrieve_response_code( $request );
 
 	if ( 200 === $request_code ) {
 		$request_body = wp_remote_retrieve_body( $request );
 		$request_data = json_decode( $request_body );
 
-		wp_enqueue_style( 'jetpack-gist-styling', $request_data->stylesheet );
+		wp_enqueue_style( 'jetpack-gist-styling', esc_url( $request_data->stylesheet ), array(), JETPACK__VERSION );
 
-		$gist = substr_replace( $request_data->div, sprintf( 'style="tab-size: %1$s" ', absint( $tab_size ) ), 5, 0 );
+		// Add inline styles for the tab style in the opening div of the gist.
+		$gist = preg_replace(
+			'#(\<div\s)+(id=\"gist[0-9]+\")+(\sclass=\"gist\"\>)?#',
+			sprintf( '$1style="tab-size: %1$s" $2$3', absint( $tab_size ) ),
+			$request_data->div,
+			1
+		);
 
-		// inline style to prevent the bottom margin to the embed that themes like TwentyTen, et al., add to tables.
+		// Add inline style to prevent the bottom margin to the embed that themes like TwentyTen, et al., add to tables.
 		$return = sprintf( '<style>.gist table { margin-bottom: 0; }</style>%1$s', $gist );
+
 	}
 
 	if (

[Aurorum]

Thank you - I've updated the PR to use that approach instead! :)

[jeherve]

This looks good to me, but I'd appreciate an additional pair of eyes on this as I've contributed code to that PR.

[jeherve]

r212543-wpcom