From 08611abba88bd8d34f9802baa12eefba95947e02 Mon Sep 17 00:00:00 2001 From: Eric Bidelman Date: Tue, 7 Feb 2017 10:03:31 -0800 Subject: [PATCH] Add inline image preview to table formatter (#1636) --- .../dobetterweb/uses-optimized-images.js | 5 ++- .../dobetterweb/uses-responsive-images.js | 10 ++++- .../formatters/partials/table.html | 38 +++++++++---------- lighthouse-core/formatters/table.js | 17 +++++++-- .../gather/gatherers/image-usage.js | 3 +- lighthouse-core/report/report-generator.js | 8 +++- .../dobetterweb/uses-optimized-images-test.js | 2 +- .../test/formatter/table-formatter-test.js | 12 ++++-- lighthouse-core/test/report/report-test.js | 10 +++-- 9 files changed, 68 insertions(+), 37 deletions(-) diff --git a/lighthouse-core/audits/dobetterweb/uses-optimized-images.js b/lighthouse-core/audits/dobetterweb/uses-optimized-images.js index 92ba129789ec..9bf93bdd9c21 100644 --- a/lighthouse-core/audits/dobetterweb/uses-optimized-images.js +++ b/lighthouse-core/audits/dobetterweb/uses-optimized-images.js @@ -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) { @@ -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 @@ -143,6 +143,7 @@ class UsesOptimizedImages extends Audit { value: { results, tableHeadings: { + preview: '', url: 'URL', total: 'Original (KB)', webpSavings: 'WebP Savings (%)', diff --git a/lighthouse-core/audits/dobetterweb/uses-responsive-images.js b/lighthouse-core/audits/dobetterweb/uses-responsive-images.js index a2a983f959f9..1c6356c6ac10 100644 --- a/lighthouse-core/audits/dobetterweb/uses-responsive-images.js +++ b/lighthouse-core/audits/dobetterweb/uses-responsive-images.js @@ -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; }, []); @@ -142,6 +149,7 @@ class UsesResponsiveImages extends Audit { value: { results, tableHeadings: { + preview: '', url: 'URL', totalKb: 'Original (KB)', potentialSavings: 'Potential Savings (%)' diff --git a/lighthouse-core/formatters/partials/table.html b/lighthouse-core/formatters/partials/table.html index bf9961713fce..1cc4a401c969 100644 --- a/lighthouse-core/formatters/partials/table.html +++ b/lighthouse-core/formatters/partials/table.html @@ -1,9 +1,13 @@ @@ -60,7 +56,9 @@ - {{#each headings}}{{/each}} + {{#each ../tableHeadings}} + + {{/each}} diff --git a/lighthouse-core/formatters/table.js b/lighthouse-core/formatters/table.js index 713d3ab42ed5..1fd9855e8c8e 100644 --- a/lighthouse-core/formatters/table.js +++ b/lighthouse-core/formatters/table.js @@ -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'; }); @@ -68,6 +70,8 @@ class Table extends Formatter { * @param {!Object} 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 @@ -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': diff --git a/lighthouse-core/gather/gatherers/image-usage.js b/lighthouse-core/gather/gatherers/image-usage.js index cc2bdbc6f10a..1dd18fbaf460 100644 --- a/lighthouse-core/gather/gatherers/image-usage.js +++ b/lighthouse-core/gather/gatherers/image-usage.js @@ -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 }; } diff --git a/lighthouse-core/report/report-generator.js b/lighthouse-core/report/report-generator.js index b8cabbbee6c7..0b1edae45c0c 100644 --- a/lighthouse-core/report/report-generator.js +++ b/lighthouse-core/report/report-generator.js @@ -134,8 +134,8 @@ class ReportGenerator { const renderer = new marked.Renderer(); renderer.em = str => `${str}`; renderer.link = (href, title, text) => { - title = title || text; - return `${text}`; + const titleAttr = title ? `title="${title}"` : ''; + return `${text}`; }; renderer.codespan = function(str) { return `${str}`; @@ -144,6 +144,10 @@ class ReportGenerator { renderer.code = function(code, language) { return `
${code}
`; }; + renderer.image = function(src, title, text) { + return `${text}`; + }; + // Nuke wrapper

tag that gets generated. renderer.paragraph = function(str) { return str; diff --git a/lighthouse-core/test/audits/dobetterweb/uses-optimized-images-test.js b/lighthouse-core/test/audits/dobetterweb/uses-optimized-images-test.js index b6ef8e6eb629..c205adcd6f44 100644 --- a/lighthouse-core/test/audits/dobetterweb/uses-optimized-images-test.js +++ b/lighthouse-core/test/audits/dobetterweb/uses-optimized-images-test.js @@ -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'); }); diff --git a/lighthouse-core/test/formatter/table-formatter-test.js b/lighthouse-core/test/formatter/table-formatter-test.js index 61cb6f7a060e..0adda23218de 100644 --- a/lighthouse-core/test/formatter/table-formatter-test.js +++ b/lighthouse-core/test/formatter/table-formatter-test.js @@ -26,7 +26,7 @@ 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, @@ -34,6 +34,7 @@ describe('TableFormatter', () => { code: 'code snippet', isEval: true, pre: 'pre snippet', + preview: {url: 'http://example.com/i.jpg', mimeType: 'image/jpeg'} }] }; @@ -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', () => { @@ -77,6 +80,7 @@ describe('TableFormatter', () => { const output = template(extendedInfo).split('\n').join(''); assert.ok(output.match('

{{this}}{{this}}
{ }; 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', () => { diff --git a/lighthouse-core/test/report/report-test.js b/lighthouse-core/test/report/report-test.js index 9b5560508bf3..97c7b5048684 100644 --- a/lighthouse-core/test/report/report-test.js +++ b/lighthouse-core/test/report/report-test.js @@ -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 () and html should not.' + helpText: '`Code like this` and [links](http://example.com) ' + + 'should be transformed. but images () ' + + 'and html should not. ' + + '[![Image preview](http://imagelink.com "Image preview")](http://imagelink.com)' }; modifiedResults.audits['bad-actor-audit-name'] = item; @@ -131,8 +133,10 @@ describe('Report', () => { assert.ok(html.includes('but renders code'), 'code blocks transformed'); assert.ok(html.includes('Code like this'), 'code blocks transformed'); assert.ok(html.includes( - 'links'), + ''), 'non-recognized HTML is sanitized'); });