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
Implements amp-analytics variables, csi pings for page load metrics. #13015
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.
LGTM
src/service/performance-impl.js
Outdated
// Store certain page visibility metrics to be exposed as analytics variables. | ||
const storedVal = Math.round(opt_delta != null ? Math.max(opt_delta, 0) | ||
: value - this.initTime_); | ||
if (label == 'fcp') { |
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.
I would make this a switch as it'll be easier to read
@keithwrightbos @gskrell This PR seems to have broken Perhaps the tests were not run recently enough? |
It's a merge issue with
5fb3da2#diff-775f4cb823bb027213fbff02cfb81778
That one removed the expandAsync() function :(
…On Thu, Jan 25, 2018 at 3:36 PM, Raghu Simha ***@***.***> wrote:
@keithwrightbos <https://github.com/keithwrightbos> @gskrell
<https://github.com/gskrell> This PR seems to have broken gulp lint on
master. https://travis-ci.org/ampproject/amphtml/jobs/333422431#L1698
Perhaps the tests were not run recently enough?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#13015 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACV2BzgCgBmyAvyW-999Vq3DMy25Vkwqks5tOOXTgaJpZM4Rr1Xb>
.
|
…mpproject#13015) * Prototype of page load amp analytics variables. * Lint fixes for page load metrics. * Change if statements to switch statement.
…mpproject#13015) * Prototype of page load amp analytics variables. * Lint fixes for page load metrics. * Change if statements to switch statement.
…mpproject#13015) * Prototype of page load amp analytics variables. * Lint fixes for page load metrics. * Change if statements to switch statement.
Follow up to #11378. Implements amp-analytics variables for Make Body Visible, First Contentful Paint, and First Viewport Ready.