Skip to content
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

Offline commenting #92

Merged
merged 24 commits into from
Oct 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
7c74cba
Add workbox background sync for comments.
miina Oct 17, 2018
93f5c5f
Fix phpcs.
miina Oct 17, 2018
44b7747
Add workbox background sync for comments.
miina Oct 17, 2018
ca07c6d
Fix phpcs.
miina Oct 17, 2018
f6472eb
Changes with rebasing the PR.
miina Oct 18, 2018
3cd1a33
Resolve conflicts.
miina Oct 18, 2018
7d67528
Add custom message for commenting.
miina Oct 18, 2018
61b7b7f
Merge remote-tracking branch 'origin/master' into feature/offline-com…
miina Oct 19, 2018
df6b0dc
Update error messages.
miina Oct 19, 2018
4e7381c
Add todo.
miina Oct 19, 2018
20bba99
Create separate component for Offline Editing.
miina Oct 23, 2018
8a62653
Merge master & resolve conflicts.
miina Oct 23, 2018
08250a5
Start adding script for AMP Live List.
miina Oct 24, 2018
78a7dcb
Modify sending response.
miina Oct 25, 2018
4da1a3f
Use regex for default offline commenting route.
miina Oct 26, 2018
69eb395
Flatten commentHandler
westonruter Oct 26, 2018
a3ce4b6
Prevent default comment handler from also matching AMP comment handler
westonruter Oct 26, 2018
138b788
Add Access-Control-Allow-Origin response header
westonruter Oct 26, 2018
c4c88b3
Fix promise chain (undo 69eb3951); use 202 status, follow redirects, …
westonruter Oct 27, 2018
de7b2eb
Use response with message property instead of error property
westonruter Oct 27, 2018
e631395
Remove todo comment.
miina Oct 30, 2018
9594969
Merge master.
miina Oct 30, 2018
8c5dd63
Remove redundant .then from fetch and only catch errors.
miina Oct 30, 2018
544a747
Merge comments component back into navigation component.
miina Oct 31, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .jshintignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ wp-includes/js/service-worker.js
wp-includes/js/service-worker-navigation-routing.js
wp-includes/js/service-worker-precaching.js
wp-includes/js/service-worker-stream-combiner.js
amp/amp-service-worker-offline-commenting.js
wp-includes/js/service-worker-offline-commenting.js
45 changes: 45 additions & 0 deletions amp/amp-service-worker-offline-commenting.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/* global ERROR_MESSAGES, SITE_URL */
{
const queue = new wp.serviceWorker.backgroundSync.Queue( 'amp-wpPendingComments' );
const errorMessages = ERROR_MESSAGES;

const commentHandler = ( { event } ) => {
const clonedRequest = event.request.clone();
return fetch( event.request ).catch( () => {
return clonedRequest.blob().then( ( body ) => {
const queuedRequest = new Request( event.request.url, {
method: event.request.method,
headers: event.request.headers,
mode: event.request.mode,
credentials: event.request.credentials,
referrer: event.request.referrer,
redirect: 'follow',
body: body
} );

// Add request to queue. @todo Replace when upgrading to Workbox v4!
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This todo can now be todone due to #98

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we'll have to wait until the related PR gets merged, too.

queue.addRequest( queuedRequest );

const jsonBody = JSON.stringify( { 'message': errorMessages.comment } );
return new Response( jsonBody, {
status: 202,
statusText: 'Accepted',
headers: {
'Access-Control-Allow-Origin': SITE_URL,
'Access-Control-Allow-Credentials': 'true',
'Content-Type': 'application/json; charset=UTF-8',
'Access-Control-Expose-Headers': 'AMP-Access-Control-Allow-Source-Origin',
'AMP-Access-Control-Allow-Source-Origin': SITE_URL,
'Cache-Control': 'no-cache, must-revalidate, max-age=0'
}
} );
} );
} );
};

wp.serviceWorker.routing.registerRoute(
/\/wp-comments-post\.php\?.*_wp_amp_action_xhr_converted.*/,
commentHandler,
'POST'
);
}
38 changes: 38 additions & 0 deletions amp/class-amp-service-worker.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public function init() {
add_action( 'wp', array( $this, 'add_install_hooks' ) );
add_action( 'wp_front_service_worker', array( $this, 'add_amp_runtime_caching' ) );
add_action( 'wp_front_service_worker', array( $this, 'add_image_runtime_caching' ) );
add_action( 'wp_front_service_worker', array( $this, 'add_live_list_offline_commenting' ) );
}

/**
Expand Down Expand Up @@ -124,6 +125,43 @@ public function add_image_runtime_caching( $service_workers ) {
);
}

/**
* Add live list offline commenting service worker script.
*
* @param object $service_workers WP Service Workers object.
*/
public function add_live_list_offline_commenting( $service_workers ) {
if ( ! ( $service_workers instanceof WP_Service_Worker_Scripts ) ) {
_doing_it_wrong( __METHOD__, esc_html__( 'Expected argument to be WP_Service_Worker_Scripts.', 'amp' ), '1.0' );
return;
}

$theme_support = AMP_Theme_Support::get_theme_support_args();
if ( empty( $theme_support['comments_live_list'] ) ) {
return;
}

$service_workers->register(
'amp-offline-commenting',
function() {
$js = file_get_contents( dirname( __FILE__ ) . '/amp-service-worker-offline-commenting.js' ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents, WordPress.WP.AlternativeFunctions.file_system_read_file_get_contents
$js = preg_replace( '#/\*\s*global.+?\*/#', '', $js );
$js = str_replace(
'ERROR_MESSAGES',
wp_json_encode( wp_service_worker_get_error_messages() ),
$js
);
$js = str_replace(
'SITE_URL',
wp_json_encode( site_url() ),
$js
);
return $js;
}
);

}

/**
* Register URLs that will be precached in the runtime cache. (Yes, this sounds somewhat strange.)
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -358,11 +358,19 @@ public function serve( WP_Service_Worker_Scripts $scripts ) {
);
}

$scripts->register(
'wp-offline-commenting',
array(
'src' => array( $this, 'get_offline_commenting_script' ),
'deps' => array( 'wp-base-config' ),
)
);

$scripts->register(
'wp-navigation-routing',
array(
'src' => array( $this, 'get_script' ),
'deps' => array( 'wp-base-config' ),
'deps' => array( 'wp-base-config', 'wp-offline-commenting' ),
)
);

Expand Down Expand Up @@ -424,6 +432,7 @@ public function serve( WP_Service_Worker_Scripts $scripts ) {
'BLACKLIST_PATTERNS' => wp_service_worker_json_encode( $this->get_blacklist_patterns() ),
'SHOULD_STREAM_RESPONSE' => wp_service_worker_json_encode( $should_stream_response ),
'STREAM_HEADER_FRAGMENT_QUERY_VAR' => wp_service_worker_json_encode( self::STREAM_FRAGMENT_QUERY_VAR ),
'ERROR_MESSAGES' => wp_service_worker_json_encode( wp_service_worker_get_error_messages() ),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@westonruter: Continued with the PR and addressed your comments, too.

Question about wp_service_worker_get_error_messages() -- do you think this method is necessary as a global method or we could just filter it here within Navigation Component?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making it a method of the navigation routing component would probably be best.

);
}

Expand Down Expand Up @@ -466,7 +475,7 @@ public function get_blacklist_patterns() {
* @return int Hook priority. A higher number means a lower priority.
*/
public function get_priority() {
return -99999;
return 99;
}

/**
Expand All @@ -484,4 +493,20 @@ public function get_script() {
$script
);
}

/**
* Get script for offline commenting requests.
*
* @return string Script.
*/
public function get_offline_commenting_script() {
$script = file_get_contents( PWA_PLUGIN_DIR . '/wp-includes/js/service-worker-offline-commenting.js' ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents
$script = preg_replace( '#/\*\s*global.+?\*/#', '', $script );

return str_replace(
array_keys( $this->replacements ),
array_values( $this->replacements ),
$script
);
}
}
Loading