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

core(responsiveness): add element screenshot to INP diagnostic #13984

Merged
merged 4 commits into from
May 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
50 changes: 38 additions & 12 deletions lighthouse-core/audits/work-during-interaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ const UIStrings = {
* @example {mousedown} interactionType
*/
displayValue: `{timeInMs, number, milliseconds}\xa0ms spent on event '{interactionType}'`,
/** Label for a column in a data table; entries will the UI element that was the target of a user interaction (for example, a button that was clicked on). Ideally fits within a ~40 character limit. */
eventTarget: 'Event target',
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);
Expand All @@ -59,7 +61,7 @@ class WorkDuringInteraction extends Audit {
description: str_(UIStrings.description),
scoreDisplayMode: Audit.SCORING_MODES.NUMERIC,
supportedModes: ['timespan'],
requiredArtifacts: ['traces', 'devtoolsLogs'],
requiredArtifacts: ['traces', 'devtoolsLogs', 'TraceElements'],
};
}

Expand Down Expand Up @@ -126,7 +128,7 @@ class WorkDuringInteraction extends Audit {
* @param {Array<LH.Artifacts.NetworkRequest>} networkRecords
* @return {{table: LH.Audit.Details.Table, phases: Record<string, {startTs: number, endTs: number}>}}
*/
static eventThreadBreakdown(interactionEvent, trace, processedTrace, networkRecords) {
static getThreadBreakdownTable(interactionEvent, trace, processedTrace, networkRecords) {
// Limit to interactionEvent's thread.
// TODO(bckenny): limit to interactionEvent's navigation.
const threadEvents = TraceProcessor.filteredTraceSort(trace.traceEvents, evt => {
Expand Down Expand Up @@ -197,6 +199,23 @@ class WorkDuringInteraction extends Audit {
};
}

/**
* @param {LH.Artifacts['TraceElements']} traceElements
* @return {LH.Audit.Details.Table | undefined}
*/
static getTraceElementTable(traceElements) {
const responsivenessElement = traceElements.find(el => el.traceEventType === 'responsiveness');
if (!responsivenessElement) return;

/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'node', itemType: 'node', text: str_(UIStrings.eventTarget)},
];
const elementItems = [{node: Audit.makeNodeItem(responsivenessElement.node)}];

return Audit.makeTableDetails(headings, elementItems);
}

/**
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
Expand Down Expand Up @@ -224,27 +243,34 @@ class WorkDuringInteraction extends Audit {
);
}

const auditDetailsItems = [];

const traceElementItem = WorkDuringInteraction.getTraceElementTable(artifacts.TraceElements);
if (traceElementItem) auditDetailsItems.push(traceElementItem);

const devtoolsLog = artifacts.devtoolsLogs[WorkDuringInteraction.DEFAULT_PASS];
// Network records will usually be empty for timespans.
const networkRecords = await NetworkRecords.request(devtoolsLog, context);
const processedTrace = await ProcessedTrace.request(trace, context);
const {table, phases} = WorkDuringInteraction.eventThreadBreakdown(
interactionEvent, trace, processedTrace, networkRecords);
const {table: breakdownTable, phases} = WorkDuringInteraction.getThreadBreakdownTable(
interactionEvent, trace, processedTrace, networkRecords);
auditDetailsItems.push(breakdownTable);

const duration = interactionEvent.args.data.duration;
const interactionType = interactionEvent.args.data.type;
const displayValue = str_(UIStrings.displayValue, {timeInMs: duration, interactionType});
auditDetailsItems.push({
type: /** @type {const} */ ('debugdata'),
interactionType,
phases,
});

const duration = interactionEvent.args.data.duration;
const displayValue = str_(UIStrings.displayValue, {timeInMs: duration, interactionType});
return {
score: duration < inpThresholds.p10 ? 1 : 0,
displayValue,
details: {
...table,
debugData: {
type: 'debugdata',
interactionType,
phases,
},
type: 'list',
items: auditDetailsItems,
},
};
}
Expand Down
23 changes: 21 additions & 2 deletions lighthouse-core/gather/gatherers/trace-elements.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const Trace = require('./trace.js');
const ProcessedTrace = require('../../computed/processed-trace.js');
const ProcessedNavigation = require('../../computed/processed-navigation.js');
const LighthouseError = require('../../lib/lh-error.js');
const ComputedResponsivenes = require('../../computed/metrics/responsiveness.js');

/** @typedef {{nodeId: number, score?: number, animations?: {name?: string, failureReasonsMask?: number, unsupportedProperties?: string[]}[]}} TraceElementData */

Expand Down Expand Up @@ -142,6 +143,23 @@ class TraceElements extends FRGatherer {
return topFive;
}

/**
* @param {LH.Trace} trace
* @param {LH.Gatherer.FRTransitionalContext} context
* @return {Promise<TraceElementData|undefined>}
*/
static async getResponsivenessElement(trace, context) {
Copy link
Collaborator

@connorjclark connorjclark May 11, 2022

Choose a reason for hiding this comment

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

nit: getResponsivenessElement -> getResponsivenessElements

Copy link
Member Author

Choose a reason for hiding this comment

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

nit: getResponsivenessElement -> getResponsivenessElements

Maybe I'll split the difference and keep the singular name but change it to return a single TraceElementData, then the caller can wrap it in an array like the lcp node id

const {settings} = context;
try {
const responsivenessEvent = await ComputedResponsivenes.request({trace, settings}, context);
if (!responsivenessEvent || responsivenessEvent.name === 'FallbackTiming') return;
return {nodeId: responsivenessEvent.args.data.nodeId};
} catch {
// Don't let responsiveness errors sink the rest of the gatherer.
return;
}
}

/**
* Find the node ids of elements which are animated using the Animation trace events.
* @param {Array<LH.TraceEvent>} mainThreadEvents
Expand Down Expand Up @@ -237,14 +255,15 @@ class TraceElements extends FRGatherer {

const lcpNodeId = largestContentfulPaintEvt?.args?.data?.nodeId;
const clsNodeData = TraceElements.getTopLayoutShiftElements(mainThreadEvents);
const animatedElementData =
await this.getAnimatedElements(mainThreadEvents);
const animatedElementData = await this.getAnimatedElements(mainThreadEvents);
const responsivenessElementData = await TraceElements.getResponsivenessElement(trace, context);

/** @type {Map<string, TraceElementData[]>} */
const backendNodeDataMap = new Map([
['largest-contentful-paint', lcpNodeId ? [{nodeId: lcpNodeId}] : []],
['layout-shift', clsNodeData],
['animation', animatedElementData],
['responsiveness', responsivenessElementData ? [responsivenessElementData] : []],
]);

const traceElements = [];
Expand Down
Loading