-
Notifications
You must be signed in to change notification settings - Fork 384
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
Add amp_print_analytics action triggering before printing entries as amp-analytics #3106
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@westonruter Question regarding Reader mode: Does the amp-analytics
extension get loaded there automatically? I was under the assumption that, so far, it only was if there were actual Analytics entries in the AMP plugin's option. However, now that there is the hook, there might be a need for the extension even without entries from the option.
Can you clarify?
@felixarntz Excellent point. Currently if the
amp-wp/includes/admin/functions.php Lines 154 to 171 in 7feaf96
amp-wp/includes/amp-post-template-functions.php Lines 121 to 132 in 7feaf96
So you will need to make sure that Site Kit does: add_filter(
'amp_post_template_data',
function( $data ) {
$data['amp_component_scripts'] = array_merge(
$data['amp_component_scripts'],
array(
'amp-analytics' => true,
)
);
return $data;
}
); This will be needed until the entire Reader mode templates are sent through the sanitizers, at which point the required components will be automatically detected. See #2202. |
For reader mode, Site Kit already covers that. How about AMP Stories though? How do I make sure the extension gets loaded in the AMP Stories template? That happens by the sanitizer too I assume? |
Correct. The AMP Stories single template is post-processed in its entirety, so any component script requirements are auto-detected. |
This is useful for printing additional
amp-analytics
tags to the page without having to refactor any existing markup generation logic to use the data structure mutated by theamp_analytics_entries
filter. For such cases, this action should be used for printingamp-analytics
tags as opposed to using thewp_footer
andamp_post_template_footer
actions; this will ensure analytics will also be included on AMP Stories.See: