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.

Sign up for GitHub

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: remove layout-shift-elements audit #15931

Merged
merged 2 commits into from
Apr 9, 2024
Merged

core: remove layout-shift-elements audit #15931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
23976fe
core: remove layout-shift-elements audit
connorjclark Apr 8, 2024
b1e7ca1
fix test
connorjclark Apr 9, 2024
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: 1 addition & 1 deletion cli/test/fixtures/perf/delayed-element.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function stall(ms) {
}

// largest-contentful-paint-element: add the largest element later in page load
// layout-shift-elements: shift down the `<h1>` in the page
// layout-shifts: shift down the `<h1>` in the page
setTimeout(() => {
const imgEl = document.createElement('img');
imgEl.src = '../dobetterweb/lighthouse-480x318.jpg';
Expand Down
50 changes: 5 additions & 45 deletions cli/test/smokehouse/test-definitions/perf-trace-elements.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,37 +80,6 @@ const expectations = {
},
type: 'image',
},
{
traceEventType: 'layout-shift-element',
node: {
selector: 'body > h1',
nodeLabel: 'Please don\'t move me',
snippet: '<h1>',
boundingRect: {
top: 465,
bottom: 502,
left: 8,
right: 404,
width: 396,
height: 37,
},
},
},
{
traceEventType: 'layout-shift-element',
node: {
nodeLabel: 'Sorry!',
snippet: '<div style="height: 18px;">',
boundingRect: {
top: 426,
bottom: 444,
left: 8,
right: 404,
width: 396,
height: 18,
},
},
},
{
traceEventType: 'layout-shift',
node: {
Expand Down Expand Up @@ -188,9 +157,9 @@ const expectations = {
],
},
},
'layout-shift-elements': {
'layout-shifts': {
score: 1,
displayValue: '2 elements found',
displayValue: '2 layout shifts found',
details: {
items: [
{
Expand All @@ -207,22 +176,13 @@ const expectations = {
height: 37,
},
},
score: '0.035 +/- 0.01',
score: '0.05 +/- 0.01',
},
{
node: {
nodeLabel: 'Sorry!',
snippet: '<div style="height: 18px;">',
boundingRect: {
top: 426,
bottom: 444,
left: 8,
right: 404,
width: 396,
height: 18,
},
nodeLabel: /Sorry|Please don't move me/,
},
score: '0.017 +/- 0.01',
score: '0.001 +/- 0.005',
},
],
},
Expand Down
101 changes: 0 additions & 101 deletions core/audits/layout-shift-elements.js
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions core/config/default-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,6 @@ const defaultConfig = {
'third-party-facades',
'largest-contentful-paint-element',
'lcp-lazy-loaded',
'layout-shift-elements',
'layout-shifts',
'long-tasks',
'non-composited-animations',
Expand Down Expand Up @@ -483,7 +482,6 @@ const defaultConfig = {
{id: 'final-screenshot', weight: 0, group: 'hidden'},
{id: 'script-treemap-data', weight: 0, group: 'hidden'},
{id: 'resource-summary', weight: 0, group: 'hidden'},
{id: 'layout-shift-elements', weight: 0, group: 'hidden'},
],
},
'accessibility': {
Expand Down
19 changes: 0 additions & 19 deletions core/gather/gatherers/trace-elements.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import {TraceEngineResult} from '../../computed/trace-engine-result.js';

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

const MAX_LAYOUT_SHIFT_ELEMENTS = 15;
const MAX_LAYOUT_SHIFTS = 15;

/**
Expand Down Expand Up @@ -66,22 +65,6 @@ class TraceElements extends BaseGatherer {
if (name) this.animationIdToName.set(id, name);
}

/**
* This function finds the top (up to 15) elements that shift on the page.
*
* @param {LH.Trace} trace
* @param {LH.Gatherer.Context} context
* @return {Promise<Array<{nodeId: number}>>}
*/
static async getTopLayoutShiftElements(trace, context) {
const {impactByNodeId} = await CumulativeLayoutShift.request(trace, context);

return [...impactByNodeId.entries()]
.sort((a, b) => b[1] - a[1])
.slice(0, MAX_LAYOUT_SHIFT_ELEMENTS)
.map(([nodeId]) => ({nodeId}));
}

/**
* We want to a single representative node to represent the shift, so let's pick
* the one with the largest impact (size x distance moved).
Expand Down Expand Up @@ -338,7 +321,6 @@ class TraceElements extends BaseGatherer {
const {mainThreadEvents} = processedTrace;

const lcpNodeData = await TraceElements.getLcpElement(trace, context);
const shiftElementsNodeData = await TraceElements.getTopLayoutShiftElements(trace, context);
const shiftsData = await TraceElements.getTopLayoutShifts(
trace, traceEngineResult, rootCauses, context);
const animatedElementData = await this.getAnimatedElements(mainThreadEvents);
Expand All @@ -347,7 +329,6 @@ class TraceElements extends BaseGatherer {
/** @type {Map<string, TraceElementData[]>} */
const backendNodeDataMap = new Map([
['largest-contentful-paint', lcpNodeData ? [lcpNodeData] : []],
['layout-shift-element', shiftElementsNodeData],
['layout-shift', shiftsData],
['animation', animatedElementData],
['responsiveness', responsivenessElementData ? [responsivenessElementData] : []],
Expand Down
Loading
Loading