-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Report v2: Basic table formatter #2019
Changes from all commits
97c38dd
d7dac3a
a9ad5ca
6a3ca90
531a460
6505c69
9a3e79f
103fe62
b2b18fe
f12c6a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,6 +56,65 @@ class Audit { | |
}); | ||
} | ||
|
||
/** | ||
* @param {!Audit.Headings} headings | ||
* @return {!Array<string>} | ||
*/ | ||
static makeV1TableHeadings(headings) { | ||
const tableHeadings = {}; | ||
headings.forEach(heading => tableHeadings[heading.key] = heading.text); | ||
return tableHeadings; | ||
} | ||
|
||
/** | ||
* Table cells will use the type specified in headings[x].itemType. However a custom type | ||
* can be provided: results[x].propName = {type: 'code', text: '...'} | ||
* @param {!Audit.Headings} headings | ||
* @param {!Array<!Object<string, *>>} results | ||
* @return {!Array<!DetailsRenderer.DetailsJSON>} | ||
*/ | ||
static makeV2TableRows(headings, results) { | ||
const tableRows = results.map(item => { | ||
return headings.map(heading => { | ||
const value = item[heading.key]; | ||
if (typeof value === 'object' && value.type) return value; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. results There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
|
||
return { | ||
type: heading.itemType, | ||
text: value | ||
}; | ||
}); | ||
}); | ||
return tableRows; | ||
} | ||
|
||
/** | ||
* @param {!Audit.Headings} headings | ||
* @return {!Array<!DetailsRenderer.DetailsJSON>} | ||
*/ | ||
static makeV2TableHeaders(headings) { | ||
return headings.map(heading => ({ | ||
type: 'text', | ||
text: heading.text | ||
})); | ||
} | ||
|
||
/** | ||
* @param {!Audit.Headings} headings | ||
* @param {!Array<!Object<string, string>>} results | ||
* @return {!DetailsRenderer.DetailsJSON} | ||
*/ | ||
static makeV2TableDetails(headings, results) { | ||
const v2TableHeaders = Audit.makeV2TableHeaders(headings); | ||
const v2TableRows = Audit.makeV2TableRows(headings, results); | ||
return { | ||
type: 'table', | ||
header: 'View Details', | ||
itemHeaders: v2TableHeaders, | ||
items: v2TableRows | ||
}; | ||
} | ||
|
||
/** | ||
* @param {!Audit} audit | ||
* @param {!AuditResult} result | ||
|
@@ -97,3 +156,21 @@ class Audit { | |
} | ||
|
||
module.exports = Audit; | ||
|
||
/** @typedef { | ||
* !Array<{ | ||
* key: string, | ||
* itemType: string, | ||
* text: string, | ||
* }>} | ||
*/ | ||
Audit.Headings; // eslint-disable-line no-unused-expressions | ||
|
||
/** @typedef {{ | ||
* results: !Array<!Object<string, string>>, | ||
* headings: !Audit.Headings, | ||
* passes: boolean, | ||
* debugString: (string|undefined) | ||
* }} | ||
*/ | ||
Audit.HeadingsResult; // eslint-disable-line no-unused-expressions |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -83,6 +83,7 @@ class OffscreenImages extends ByteEfficiencyAudit { | |
return { | ||
url, | ||
preview: { | ||
type: 'thumbnail', | ||
url: image.networkRecord.url, | ||
mimeType: image.networkRecord.mimeType | ||
}, | ||
|
@@ -95,8 +96,7 @@ class OffscreenImages extends ByteEfficiencyAudit { | |
|
||
/** | ||
* @param {!Artifacts} artifacts | ||
* @return {{results: !Array<Object>, tableHeadings: Object, | ||
* passes: boolean=, debugString: string=}} | ||
* @return {!Audit.HeadingsResult} | ||
*/ | ||
static audit_(artifacts) { | ||
const images = artifacts.ImageUsage; | ||
|
@@ -132,15 +132,18 @@ class OffscreenImages extends ByteEfficiencyAudit { | |
const loadedEarly = item.requestStartTime < ttiTimestamp; | ||
return isWasteful && loadedEarly; | ||
}); | ||
|
||
const headings = [ | ||
{key: 'preview', itemType: 'thumbnail', text: ''}, | ||
{key: 'url', itemType: 'url', text: 'URL'}, | ||
{key: 'totalKb', itemType: 'text', text: 'Original'}, | ||
{key: 'potentialSavings', itemType: 'text', text: 'Potential Savings'}, | ||
]; | ||
|
||
return { | ||
debugString, | ||
results, | ||
tableHeadings: { | ||
preview: '', | ||
url: 'URL', | ||
totalKb: 'Original', | ||
potentialSavings: 'Potential Savings', | ||
} | ||
headings, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
}; | ||
}); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -164,8 +164,7 @@ class UnusedCSSRules extends ByteEfficiencyAudit { | |
|
||
/** | ||
* @param {!Artifacts} artifacts | ||
* @return {{results: !Array<Object>, tableHeadings: Object, | ||
* passes: boolean=, debugString: string=}} | ||
* @return {{results: !Array<Object>, headings: !Audit.Headings}} | ||
*/ | ||
static audit_(artifacts) { | ||
const styles = artifacts.Styles; | ||
|
@@ -179,14 +178,17 @@ class UnusedCSSRules extends ByteEfficiencyAudit { | |
return UnusedCSSRules.mapSheetToResult(indexedSheets[sheetId], pageUrl); | ||
}).filter(sheet => sheet && sheet.wastedBytes > 1024); | ||
|
||
|
||
const headings = [ | ||
{key: 'url', itemType: 'url', text: 'URL'}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can override this per row correct? it'd be nice to finally have a nice solution for displaying the inline code preview in the url column There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to have an individual row change the itemType? that seems a little weird. whyd we want diff itemtypes in the column? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in all the css-based ones the "url column" doesn't always have a url and it's a code snippet preview instead since it's an inline style. it'd be nice to have the details renderer support that too so the code snippets can say |
||
{key: 'numUnused', itemType: 'url', text: 'Unused Rules'}, | ||
{key: 'totalKb', itemType: 'text', text: 'Original'}, | ||
{key: 'potentialSavings', itemType: 'text', text: 'Potential Savings'}, | ||
]; | ||
|
||
return { | ||
results, | ||
tableHeadings: { | ||
url: 'URL', | ||
numUnused: 'Unused Rules', | ||
totalKb: 'Original', | ||
potentialSavings: 'Potential Savings', | ||
} | ||
headings | ||
}; | ||
} | ||
} | ||
|
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.
🎉