Skip to content

Commit

Permalink
core(report): show nodeLabel for DOM nodes in addition to snippet (#8961
Browse files Browse the repository at this point in the history
)
  • Loading branch information
mattzeunert authored and paulirish committed May 28, 2019
1 parent 9dc0360 commit 129e367
Show file tree
Hide file tree
Showing 15 changed files with 562 additions and 190 deletions.
387 changes: 303 additions & 84 deletions lighthouse-cli/test/smokehouse/a11y/expectations.js

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions lighthouse-cli/test/smokehouse/seo/expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ module.exports = [
'\n too small target\n </a>',
'path': '2,HTML,1,BODY,3,DIV,21,DIV,0,A',
'selector': 'body > div > div > a',
'nodeLabel': 'too small target',
},
'overlappingTarget': {
'type': 'node',
Expand All @@ -213,6 +214,7 @@ module.exports = [
'\n big enough target\n </a>',
'path': '2,HTML,1,BODY,3,DIV,21,DIV,1,A',
'selector': 'body > div > div > a',
'nodeLabel': 'big enough target',
},
'size': '100x30',
'width': 100,
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/audits/accessibility/axe-audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class AxeAudit extends Audit {
path: node.path,
snippet: node.html || node.snippet,
explanation: node.failureSummary,
nodeLabel: node.nodeLabel,
}),
}));
}
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/audits/seo/tap-targets.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ function targetToTableNode(target) {
snippet: target.snippet,
path: target.path,
selector: target.selector,
nodeLabel: target.nodeLabel,
};
}

Expand Down
5 changes: 4 additions & 1 deletion lighthouse-core/gather/gatherers/accessibility.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
'use strict';

/* global window, document, getOuterHTMLSnippet, getNodePath */
/* global window, document, getOuterHTMLSnippet, getNodePath, getNodeLabel */

const Gatherer = require('./gatherer');
const fs = require('fs');
Expand Down Expand Up @@ -57,6 +57,8 @@ function runA11yChecks() {
node.path = getNodePath(node.element);
// @ts-ignore - getOuterHTMLSnippet put into scope via stringification
node.snippet = getOuterHTMLSnippet(node.element);
// @ts-ignore - getNodeLabel put into scope via stringification
node.nodeLabel = getNodeLabel(node.element);
// avoid circular JSON concerns
node.element = node.any = node.all = node.none = undefined;
}));
Expand All @@ -77,6 +79,7 @@ class Accessibility extends Gatherer {
const expression = `(function () {
${pageFunctions.getOuterHTMLSnippetString};
${pageFunctions.getNodePathString};
${pageFunctions.getNodeLabelString};
${axeLibSource};
return (${runA11yChecks.toString()}());
})()`;
Expand Down
7 changes: 5 additions & 2 deletions lighthouse-core/gather/gatherers/seo/tap-targets.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
'use strict';

/* global getComputedStyle, getElementsInDocument, Node, getNodePath, getNodeSelector */
/* global getComputedStyle, getElementsInDocument, Node, getNodePath, getNodeSelector, getNodeLabel */

const Gatherer = require('../gatherer');
const pageFunctions = require('../../../lib/page-functions.js');
Expand Down Expand Up @@ -239,7 +239,7 @@ function elementIsPositionFixedStickyOrAbsolute(element) {
/**
* @param {string} str
* @param {number} maxLength
* @returns {string}
* @return {string}
*/
/* istanbul ignore next */
function truncate(str, maxLength) {
Expand Down Expand Up @@ -294,6 +294,8 @@ function gatherTapTargets() {
path: getNodePath(tapTargetElement),
// @ts-ignore - getNodeSelector put into scope via stringification
selector: getNodeSelector(tapTargetElement),
// @ts-ignore - getNodeLabel put into scope via stringification
nodeLabel: getNodeLabel(tapTargetElement),
href: /** @type {HTMLAnchorElement} */(tapTargetElement)['href'] || '',
});
});
Expand Down Expand Up @@ -323,6 +325,7 @@ class TapTargets extends Gatherer {
${rectContainsString};
${pageFunctions.getNodePathString};
${pageFunctions.getNodeSelectorString};
${pageFunctions.getNodeLabelString};
${gatherTapTargets.toString()};
return gatherTapTargets();
Expand Down
45 changes: 44 additions & 1 deletion lighthouse-core/lib/page-functions.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ function getNodePath(node) {

/**
* @param {Element} node
* @returns {string}
* @return {string}
*/
/* istanbul ignore next */
function getNodeSelector(node) {
Expand Down Expand Up @@ -218,6 +218,47 @@ function getNodeSelector(node) {
return parts.join(' > ');
}

/**
* Generate a human-readable label for the given element, based on end-user facing
* strings like the innerText or alt attribute.
* Falls back to the tagName if no useful label is found.
* @param {HTMLElement} node
* @return {string|null}
*/
/* istanbul ignore next */
function getNodeLabel(node) {
// Inline so that audits that import getNodeLabel don't
// also need to import truncate
/**
* @param {string} str
* @param {number} maxLength
* @return {string}
*/
function truncate(str, maxLength) {
if (str.length <= maxLength) {
return str;
}
return str.slice(0, maxLength - 1) + '…';
}

const tagName = node.tagName.toLowerCase();
// html and body content is too broad to be useful, since they contain all page content
if (tagName !== 'html' && tagName !== 'body') {
const nodeLabel = node.innerText || node.getAttribute('alt') || node.getAttribute('aria-label');
if (nodeLabel) {
return truncate(nodeLabel, 80);
} else {
// If no useful label was found then try to get one from a child.
// E.g. if an a tag contains an image but no text we want the image alt/aria-label attribute.
const nodeToUseForLabel = node.querySelector('[alt], [aria-label]');
if (nodeToUseForLabel) {
return getNodeLabel(/** @type {HTMLElement} */ (nodeToUseForLabel));
}
}
}
return tagName;
}

module.exports = {
wrapRuntimeEvalErrorInBrowserString: wrapRuntimeEvalErrorInBrowser.toString(),
registerPerformanceObserverInPageString: registerPerformanceObserverInPage.toString(),
Expand All @@ -230,4 +271,6 @@ module.exports = {
getNodePathString: getNodePath.toString(),
getNodeSelectorString: getNodeSelector.toString(),
getNodeSelector: getNodeSelector,
getNodeLabel: getNodeLabel,
getNodeLabelString: getNodeLabel.toString(),
};
11 changes: 10 additions & 1 deletion lighthouse-core/report/html/renderer/details-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -352,15 +352,24 @@ class DetailsRenderer {
*/
renderNode(item) {
const element = this._dom.createElement('span', 'lh-node');
if (item.nodeLabel) {
const nodeLabelEl = this._dom.createElement('div');
nodeLabelEl.textContent = item.nodeLabel;
element.appendChild(nodeLabelEl);
}
if (item.snippet) {
element.textContent = item.snippet;
const snippetEl = this._dom.createElement('div');
snippetEl.classList.add('lh-node__snippet');
snippetEl.textContent = item.snippet;
element.appendChild(snippetEl);
}
if (item.selector) {
element.title = item.selector;
}
if (item.path) element.setAttribute('data-path', item.path);
if (item.selector) element.setAttribute('data-selector', item.selector);
if (item.snippet) element.setAttribute('data-snippet', item.snippet);

return element;
}

Expand Down
13 changes: 9 additions & 4 deletions lighthouse-core/report/html/report-styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@
--chevron-size: 12px;
--inner-audit-left-padding: calc(var(--score-shape-size) + var(--score-shape-margin-left) + var(--score-shape-margin-right));

/* Palette. */
/* Palette using Material Design Colors
* https://www.materialui.co/colors */
--color-black-100: #F5F5F5;
--color-black-200: #E0E0E0;
--color-black-400: #BDBDBD;
Expand All @@ -100,6 +101,8 @@
--color-white: #FFFFFF;
--color-blue-200: #90CAF9;
--color-blue-900: #0D47A1;
--color-cyan-500: #00BCD4;
--color-teal-600: #00897B;


/* TODO(cjamcl) clean up unused variables. */
Expand Down Expand Up @@ -175,6 +178,7 @@
.lh-vars.dark {
--color-red-700: var(--color-red);
--color-green-700: var(--color-green);
--color-teal-600: var(--color-cyan-500);
--color-orange-700: var(--color-orange);

--color-black-200: var(--color-black-800);
Expand Down Expand Up @@ -388,10 +392,11 @@
}

/* Node */
.lh-node {
display: block;
.lh-node__snippet {
font-family: var(--monospace-font-family);
color: hsl(174, 100%, 27%);
color: var(--color-teal-600);
font-size: 12px;
line-height: 1.5em;
}

/* Score */
Expand Down
33 changes: 33 additions & 0 deletions lighthouse-core/test/lib/page-functions-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,37 @@ describe('Page Functions', () => {
assert.equal(pageFunctions.getNodeSelector(childEl), 'div#wrapper > div.child');
});
});

describe('getNodeLabel', () => {
it('Returns innerText if element has visible text', () => {
const el = dom.createElement('div');
el.innerText = 'Hello';
assert.equal(pageFunctions.getNodeLabel(el), 'Hello');
});

it('Falls back to children and alt/aria-label if a title can\'t be determined', () => {
const el = dom.createElement('div');
const childEl = dom.createElement('div', '', {'aria-label': 'Something'});
el.appendChild(childEl);
assert.equal(pageFunctions.getNodeLabel(el), 'Something');
});

it('Truncates long text', () => {
const el = dom.createElement('div');
el.setAttribute('alt', Array(100).fill('a').join(''));
assert.equal(pageFunctions.getNodeLabel(el).length, 80);
});

it('Uses tag name for html tags', () => {
const el = dom.createElement('html');
assert.equal(pageFunctions.getNodeLabel(el), 'html');
});

it('Uses tag name if there is no better label', () => {
const el = dom.createElement('div');
const childEl = dom.createElement('span');
el.appendChild(childEl);
assert.equal(pageFunctions.getNodeLabel(el), 'div');
});
});
});
Loading

0 comments on commit 129e367

Please sign in to comment.