Skip to content

Commit

Permalink
Add inline image preview to table formatter (#1636)
Browse files Browse the repository at this point in the history
  • Loading branch information
ebidel authored and paulirish committed Feb 7, 2017
1 parent c88ea11 commit 08611ab
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 37 deletions.
5 changes: 3 additions & 2 deletions lighthouse-core/audits/dobetterweb/uses-optimized-images.js
Expand Up @@ -94,7 +94,6 @@ class UsesOptimizedImages extends Audit {
}

const originalKb = Math.round(image.originalSize / KB_IN_BYTES);
const url = URL.getDisplayName(image.url);
const webpSavings = UsesOptimizedImages.computeSavings(image, 'webp');

if (webpSavings.bytes > WEBP_ALREADY_OPTIMIZED_THRESHOLD_IN_BYTES) {
Expand All @@ -112,7 +111,8 @@ class UsesOptimizedImages extends Audit {

totalWastedBytes += webpSavings.bytes;
results.push({
url,
url: URL.getDisplayName(image.url),
preview: {url: image.url, mimeType: image.mimeType},
total: `${originalKb} KB`,
webpSavings: `${webpSavings.percent}%`,
jpegSavings: jpegSavingsLabel
Expand Down Expand Up @@ -143,6 +143,7 @@ class UsesOptimizedImages extends Audit {
value: {
results,
tableHeadings: {
preview: '',
url: 'URL',
total: 'Original (KB)',
webpSavings: 'WebP Savings (%)',
Expand Down
10 changes: 9 additions & 1 deletion lighthouse-core/audits/dobetterweb/uses-responsive-images.js
Expand Up @@ -121,7 +121,14 @@ class UsesResponsiveImages extends Audit {

hasWastefulImage = hasWastefulImage || processed.isWasteful;
totalWastedBytes += processed.wastedBytes;
results.push(processed.result);

results.push(Object.assign({
preview: {
url: image.networkRecord.url,
mimeType: image.networkRecord.mimeType
}
}, processed.result));

return results;
}, []);

Expand All @@ -142,6 +149,7 @@ class UsesResponsiveImages extends Audit {
value: {
results,
tableHeadings: {
preview: '',
url: 'URL',
totalKb: 'Original (KB)',
potentialSavings: 'Potential Savings (%)'
Expand Down
38 changes: 18 additions & 20 deletions lighthouse-core/formatters/partials/table.html
@@ -1,54 +1,50 @@
<style>
.table_list {
--image-preview: 24px;
margin-top: 8px;
border: 1px solid #EBEBEB;
border-spacing: 0;
max-width: 100%;
table-layout: fixed;
}
.table_list.multicolumn {
width: 100%;
}
.table_list th,
.table_list td {
overflow: auto;
}
.table_list th {
background-color: #eee;
padding: 15px 10px;
padding: 12px 10px;
line-height: 1.2;
}
.table_list td {
padding: 10px;
}
.table_list th:first-of-type,
.table_list td:first-of-type {
min-width: 40%;
white-space: nowrap;
}
.table_list tr:nth-child(even) {
background-color: #fafafa;
}
.table_list tr:nth-child(even),
.table_list tr:hover {
background-color: #fafafa;
}
.table_list code, .table_list pre {
white-space: pre;
font-family: monospace;
display: block;
margin: 0;
}
.table_list em + code, .table_list em + pre {
margin-top: 10px;
}

.table_list.multicolumn {
display: flex;
flex-direction: column;
}
.table_list.multicolumn tr {
display: flex;
.table_list td img {
height: var(--image-preview);
width: var(--image-preview);
object-fit: contain;
}
.table_list.multicolumn th,
.table_list.multicolumn td {
display: flex;
flex-direction: column;
justify-content: center;
flex: 1;
.table_list .preview-image {
width: calc(var(--image-preview) * 2);
}
</style>

Expand All @@ -60,7 +56,9 @@
<table class="table_list {{#if_not_eq headings.length 2}}multicolumn{{/if_not_eq}}">
<thead>
<tr>
{{#each headings}}<th>{{this}}</th>{{/each}}
{{#each ../tableHeadings}}
<th {{#if_eq @key "preview"}}class="preview-image"{{/if_eq}}>{{this}}</th>
{{/each}}
</tr>
</thead>
<tbody>
Expand Down
17 changes: 13 additions & 4 deletions lighthouse-core/formatters/table.js
Expand Up @@ -43,11 +43,13 @@ class Table extends Formatter {
table.rows.forEach(row => {
output += ' ';
row.cols.forEach(col => {
// Omit code snippet cols.
if (!col || col.startsWith('`') && col.endsWith('`')) {
return;
// Omit code snippet cols and image previews.
if (!col || col.startsWith('`') && col.endsWith('`') ||
col.startsWith('[![Image preview]')) {
output += '- ';
} else {
output += `${col} `;
}
output += `${col} `;
});
output += '\n';
});
Expand All @@ -68,6 +70,8 @@ class Table extends Formatter {
* @param {!Object<string>} headings for the table. The order of this
* object's key/value pairs determines the order of the HTML table headings.
* There is special handling for certain keys:
* preview {url: string, mimeType: string}: For image mimetypes, wraps
* the value in a markdown image.
* code: wraps the value in single ` for a markdown code snippet.
* pre: wraps the value in triple ``` for a markdown code block.
* lineCol: combines the values for the line and col keys into a single
Expand All @@ -88,6 +92,11 @@ class Table extends Formatter {
}

switch (key) {
case 'preview':
if (/^image/.test(value.mimeType)) {
return `[![Image preview](${value.url} "Image preview")](${value.url})`;
}
return '';
case 'code':
return '`' + value.trim() + '`';
case 'pre':
Expand Down
3 changes: 2 additions & 1 deletion lighthouse-core/gather/gatherers/image-usage.js
Expand Up @@ -80,7 +80,8 @@ class ImageUsage extends Gatherer {
resourceSize: record.resourceSize,
startTime: record.startTime,
endTime: record.endTime,
responseReceivedTime: record.responseReceivedTime
responseReceivedTime: record.responseReceivedTime,
mimeType: record._mimeType
};
}

Expand Down
8 changes: 6 additions & 2 deletions lighthouse-core/report/report-generator.js
Expand Up @@ -134,8 +134,8 @@ class ReportGenerator {
const renderer = new marked.Renderer();
renderer.em = str => `<em>${str}</em>`;
renderer.link = (href, title, text) => {
title = title || text;
return `<a href="${href}" target="_blank" rel="noopener" title="${title}">${text}</a>`;
const titleAttr = title ? `title="${title}"` : '';
return `<a href="${href}" target="_blank" rel="noopener" ${titleAttr}>${text}</a>`;
};
renderer.codespan = function(str) {
return `<code>${str}</code>`;
Expand All @@ -144,6 +144,10 @@ class ReportGenerator {
renderer.code = function(code, language) {
return `<pre>${code}</pre>`;
};
renderer.image = function(src, title, text) {
return `<img src="${src}" alt="${text}" title="${title}">`;
};

// Nuke wrapper <p> tag that gets generated.
renderer.paragraph = function(str) {
return str;
Expand Down
Expand Up @@ -48,7 +48,7 @@ describe('Page uses optimized images', () => {

const headings = auditResult.extendedInfo.value.tableHeadings;
assert.deepEqual(Object.keys(headings).map(key => headings[key]),
['URL', 'Original (KB)', 'WebP Savings (%)', 'JPEG Savings (%)'],
['', 'URL', 'Original (KB)', 'WebP Savings (%)', 'JPEG Savings (%)'],
'table headings are correct and in order');
});

Expand Down
12 changes: 9 additions & 3 deletions lighthouse-core/test/formatter/table-formatter-test.js
Expand Up @@ -26,14 +26,15 @@ describe('TableFormatter', () => {
const extendedInfo = {
tableHeadings: {
url: 'URL', lineCol: 'Line/col', code: 'Snippet', isEval: 'Eval\'d?',
pre: 'Code'},
pre: 'Code', preview: 'Preview'},
results: [{
url: 'http://example.com',
line: 123,
col: 456,
code: 'code snippet',
isEval: true,
pre: 'pre snippet',
preview: {url: 'http://example.com/i.jpg', mimeType: 'image/jpeg'}
}]
};

Expand All @@ -56,15 +57,17 @@ describe('TableFormatter', () => {
assert.equal(table.rows[0].cols[2], '\`code snippet\`');
assert.equal(table.rows[0].cols[3], 'yes');
assert.equal(table.rows[0].cols[4], '\`\`\`\npre snippet\`\`\`');
assert.equal(table.rows[0].cols[5],
'[![Image preview](http://example.com/i.jpg "Image preview")](http://example.com/i.jpg)');
});

it('generates valid pretty output', () => {
const pretty = TableFormatter.getFormatter('pretty');
const output = pretty(extendedInfo);
assert.ok(output.includes(
' URL LINE/COL SNIPPET EVAL\'D? CODE\n'), 'prints table headings');
' URL LINE/COL SNIPPET EVAL\'D? CODE PREVIEW\n'), 'prints table headings');
assert.ok(output.includes(
' http://example.com 123:456 yes \n'), 'prints cells');
' http://example.com 123:456 - yes - - \n'), 'prints cells');
});

it('generates valid html output', () => {
Expand All @@ -77,13 +80,16 @@ describe('TableFormatter', () => {
const output = template(extendedInfo).split('\n').join('');
assert.ok(output.match('<table class="table_list'), 'creates a table');
assert.ok(output.match('multicolumn'), 'adds multicolumn class for large tables');
assert.ok(output.match('class="preview-image"'), 'renders image preview');

const extendedInfoShort = {
tableHeadings: {url: 'URL', lineCol: 'Line/col'},
results: extendedInfo.results
};
const output2 = template(extendedInfoShort).split('\n').join('');
assert.ok(!output2.match('multicolumn"'), 'does not add multicolumn class for small tables');
assert.ok(!output2.match('class="preview-image'),
'does not add preview-image class if table does not have images');
});

it('handles missing values', () => {
Expand Down
10 changes: 7 additions & 3 deletions lighthouse-core/test/report/report-test.js
Expand Up @@ -108,8 +108,10 @@ describe('Report', () => {
name: 'bad-actor-audit-name',
category: 'Fake Audit Aggregation',
description: 'Report does not inject unknown HTML but `renders code`',
helpText: '`Code like this` and [links](http://example.com) should be transformed. ' +
'but images (<img src="test.gif" onerror="alert(10)">) and <b>html should not</b>.'
helpText: '`Code like this` and [links](http://example.com) ' +
'should be transformed. but images (<img src="test.gif" onerror="alert(10)">) ' +
'and <b>html should not</b>. ' +
'[![Image preview](http://imagelink.com "Image preview")](http://imagelink.com)'
};

modifiedResults.audits['bad-actor-audit-name'] = item;
Expand All @@ -131,8 +133,10 @@ describe('Report', () => {
assert.ok(html.includes('but <code>renders code</code>'), 'code blocks transformed');
assert.ok(html.includes('<code>Code like this</code>'), 'code blocks transformed');
assert.ok(html.includes(
'<a href="http://example.com" target="_blank" rel="noopener" title="links">links</a>'),
'<a href="http://example.com" target="_blank" rel="noopener"'),
'anchors are transformed');
assert.ok(html.includes( '<a href="http://imagelink.com"'), 'images in links are transformed');
assert.ok(html.includes( '<img src="http://imagelink.com"'), 'images are transformed');
assert.ok(!html.includes(
'<img src="test.gif" onerror="alert(10)">'), 'non-recognized HTML is sanitized');
});
Expand Down

0 comments on commit 08611ab

Please sign in to comment.