Skip to content

Commit

Permalink
Avoid small gaps in collapsed snippets
Browse files Browse the repository at this point in the history
  • Loading branch information
mattzeunert committed Feb 3, 2019
1 parent f60cb7f commit e490a89
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 0 deletions.
6 changes: 6 additions & 0 deletions lighthouse-core/report/html/renderer/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,7 @@ class Util {
return lines.slice(0, surroundingLineCount * 2 + 1);
}

const minGapSize = 3;
const lineNumbersToKeep = new Set();
highlights.forEach(({lineNumber}) => {
if (!lineNumber) {
Expand All @@ -477,6 +478,11 @@ class Util {
firstSurroundingLineNumber++;
lastSurroundingLineNumber++;
}
// If we only a few lines would be omitted normally then we prefer to include
// extra lines to avoid the tiny gap
if (lineNumbersToKeep.has(firstSurroundingLineNumber - minGapSize - 1)) {
firstSurroundingLineNumber -= minGapSize;
}
for (let i = firstSurroundingLineNumber; i <= lastSurroundingLineNumber; i++) {
const surroundingLineNumber = i;
lineNumbersToKeep.add(surroundingLineNumber);
Expand Down
20 changes: 20 additions & 0 deletions lighthouse-core/test/audits/audit-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,26 @@ describe('Audit', () => {
expect(details.lines.length).toBe(2 * maxLinesAroundHighlight + 1);
});

it('Does not omit lines if fewer than 4 lines would be omitted', () => {
const details = Audit.makeSnippetListDetails([{
content: makeLines(200),
title: 'Title',
highlights: [
// without the special logic for small gaps lines 71-73 would be missing
{
lineNumber: 60,
message: 'Highlight 1',
}, {
lineNumber: 84,
message: 'Highlight 2',
}],
maxLinesAroundHighlight,
}]).items[0];

const normalExpectedLineNumber = 2 * (maxLinesAroundHighlight * 2 + 1);
assert.equal(details.lines.length, normalExpectedLineNumber + 3);
});

it('Limits the number of lines around highlights', () => {
const content = makeLines(99) + 'A\n' + makeLines(99) + '\nB';
const allLines = content.split('\n');
Expand Down

0 comments on commit e490a89

Please sign in to comment.