Skip to content

Commit

Permalink
report: make urls clickable (#9224)
Browse files Browse the repository at this point in the history
  • Loading branch information
connorjclark authored Jun 29, 2019
1 parent 8bef454 commit d9e012b
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 36 deletions.
31 changes: 18 additions & 13 deletions lighthouse-core/report/html/renderer/crc-details-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,10 @@ class CriticalRequestChainRenderer {
* @param {DOM} dom
* @param {DocumentFragment} tmpl
* @param {CRCSegment} segment
* @param {DetailsRenderer} detailsRenderer
* @return {Node}
*/
static createChainNode(dom, tmpl, segment) {
static createChainNode(dom, tmpl, segment, detailsRenderer) {
const chainsEl = dom.cloneTemplate('#tmpl-lh-crc__chains', tmpl);

// Hovering over request shows full URL.
Expand Down Expand Up @@ -120,10 +121,10 @@ class CriticalRequestChainRenderer {
}

// Fill in url, host, and request size information.
const {file, hostname} = Util.parseURL(segment.node.request.url);
const url = segment.node.request.url;
const linkEl = detailsRenderer.renderTextURL(url);
const treevalEl = dom.find('.crc-node__tree-value', chainsEl);
dom.find('.crc-node__tree-file', treevalEl).textContent = `${file}`;
dom.find('.crc-node__tree-hostname', treevalEl).textContent = hostname ? `(${hostname})` : '';
treevalEl.appendChild(linkEl);

if (!segment.hasChildren) {
const {startTime, endTime, transferSize} = segment.node.request;
Expand All @@ -146,14 +147,15 @@ class CriticalRequestChainRenderer {
* @param {CRCSegment} segment
* @param {Element} elem Parent element.
* @param {LH.Audit.Details.CriticalRequestChain} details
* @param {DetailsRenderer} detailsRenderer
*/
static buildTree(dom, tmpl, segment, elem, details) {
elem.appendChild(CriticalRequestChainRenderer.createChainNode(dom, tmpl, segment));
static buildTree(dom, tmpl, segment, elem, details, detailsRenderer) {
elem.appendChild(CRCRenderer.createChainNode(dom, tmpl, segment, detailsRenderer));
if (segment.node.children) {
for (const key of Object.keys(segment.node.children)) {
const childSegment = CriticalRequestChainRenderer.createSegment(segment.node.children, key,
const childSegment = CRCRenderer.createSegment(segment.node.children, key,
segment.startTime, segment.transferSize, segment.treeMarkers, segment.isLastChild);
CriticalRequestChainRenderer.buildTree(dom, tmpl, childSegment, elem, details);
CRCRenderer.buildTree(dom, tmpl, childSegment, elem, details, detailsRenderer);
}
}
}
Expand All @@ -162,9 +164,10 @@ class CriticalRequestChainRenderer {
* @param {DOM} dom
* @param {ParentNode} templateContext
* @param {LH.Audit.Details.CriticalRequestChain} details
* @param {DetailsRenderer} detailsRenderer
* @return {Element}
*/
static render(dom, templateContext, details) {
static render(dom, templateContext, details, detailsRenderer) {
const tmpl = dom.cloneTemplate('#tmpl-lh-crc', templateContext);
const containerEl = dom.find('.lh-crc', tmpl);

Expand All @@ -176,17 +179,19 @@ class CriticalRequestChainRenderer {
Util.formatMilliseconds(details.longestChain.duration);

// Construct visual tree.
const root = CriticalRequestChainRenderer.initTree(details.chains);
const root = CRCRenderer.initTree(details.chains);
for (const key of Object.keys(root.tree)) {
const segment = CriticalRequestChainRenderer.createSegment(root.tree, key,
root.startTime, root.transferSize);
CriticalRequestChainRenderer.buildTree(dom, tmpl, segment, containerEl, details);
const segment = CRCRenderer.createSegment(root.tree, key, root.startTime, root.transferSize);
CRCRenderer.buildTree(dom, tmpl, segment, containerEl, details, detailsRenderer);
}

return dom.find('.lh-crc-container', tmpl);
}
}

// Alias b/c the name is really long.
const CRCRenderer = CriticalRequestChainRenderer;

// Allow Node require()'ing.
if (typeof module !== 'undefined' && module.exports) {
module.exports = CriticalRequestChainRenderer;
Expand Down
12 changes: 6 additions & 6 deletions lighthouse-core/report/html/renderer/details-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class DetailsRenderer {
case 'table':
return this._renderTable(details);
case 'criticalrequestchain':
return CriticalRequestChainRenderer.render(this._dom, this._templateContext, details);
return CriticalRequestChainRenderer.render(this._dom, this._templateContext, details, this);
case 'opportunity':
return this._renderTable(details);

Expand Down Expand Up @@ -97,7 +97,7 @@ class DetailsRenderer {
* @param {string} text
* @return {HTMLElement}
*/
_renderTextURL(text) {
renderTextURL(text) {
const url = text;

let displayedPath;
Expand All @@ -113,7 +113,7 @@ class DetailsRenderer {
}

const element = this._dom.createElement('div', 'lh-text__url');
element.appendChild(this._renderText(displayedPath));
element.appendChild(this._renderLink({text: displayedPath, url}));

if (displayedHost) {
const hostElem = this._renderText(displayedHost);
Expand All @@ -130,7 +130,7 @@ class DetailsRenderer {
}

/**
* @param {LH.Audit.Details.LinkValue} details
* @param {{text: string, url: string}} details
* @return {Element}
*/
_renderLink(details) {
Expand Down Expand Up @@ -215,7 +215,7 @@ class DetailsRenderer {
return this.renderNode(value);
}
case 'url': {
return this._renderTextURL(value.value);
return this.renderTextURL(value.value);
}
default: {
throw new Error(`Unknown valueType: ${value.type}`);
Expand Down Expand Up @@ -260,7 +260,7 @@ class DetailsRenderer {
case 'url': {
const strValue = String(value);
if (URL_PREFIXES.some(prefix => strValue.startsWith(prefix))) {
return this._renderTextURL(strValue);
return this.renderTextURL(strValue);
} else {
// Fall back to <pre> rendering if not actually a URL.
return this._renderCode(strValue);
Expand Down
15 changes: 10 additions & 5 deletions lighthouse-core/report/html/report-styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -1365,12 +1365,8 @@
width: 12%;
}

.lh-text__url:hover {
text-decoration: underline dotted #999;
text-decoration-skip-ink: auto;
}

.lh-text__url > .lh-text, .lh-text__url-host {
.lh-text__url-host {
display: inline;
}

Expand All @@ -1386,6 +1382,15 @@
object-fit: contain;
}

.lh-text__url > a {
color: inherit;
text-decoration: none;
}

.lh-text__url > a:hover {
text-decoration: underline dotted #999;
}

/* Chevron
https://codepen.io/paulirish/pen/LmzEmK
*/
Expand Down
8 changes: 3 additions & 5 deletions lighthouse-core/report/html/templates.html
Original file line number Diff line number Diff line change
Expand Up @@ -741,12 +741,12 @@
.lh-crc .crc-node__tree-value {
margin-left: 10px;
}
.lh-crc .crc-node__tree-value div {
display: inline;
}
.lh-crc .crc-node__chain-duration {
font-weight: 700;
}
.lh-crc .crc-node__tree-hostname {
color: #595959;
}
.lh-crc .crc-initial-nav {
color: #595959;
font-style: italic;
Expand All @@ -769,8 +769,6 @@

</span>
<span class="crc-node__tree-value">
<span class="crc-node__tree-file"><!-- fill me: node.request.url.file --></span>
<span class="crc-node__tree-hostname">(<!-- fill me: node.request.url.host -->)</span>

</span>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const jsdom = require('jsdom');
const URL = require('../../../../lib/url-shim.js');
const Util = require('../../../../report/html/renderer/util.js');
const DOM = require('../../../../report/html/renderer/dom.js');
const DetailsRenderer = require('../../../../report/html/renderer/details-renderer.js');
const CriticalRequestChainRenderer =
require('../../../../report/html/renderer/crc-details-renderer.js');

Expand Down Expand Up @@ -75,12 +76,14 @@ const DETAILS = {

describe('DetailsRenderer', () => {
let dom;
let detailsRenderer;

beforeAll(() => {
global.URL = URL;
global.Util = Util;
const {document} = new jsdom.JSDOM(TEMPLATE_FILE).window;
dom = new DOM(document);
detailsRenderer = new DetailsRenderer(dom);
});

afterAll(() => {
Expand All @@ -89,18 +92,25 @@ describe('DetailsRenderer', () => {
});

it('renders tree structure', () => {
const el = CriticalRequestChainRenderer.render(dom, dom.document(), DETAILS);
const el = CriticalRequestChainRenderer.render(dom, dom.document(), DETAILS, detailsRenderer);
const chains = el.querySelectorAll('.crc-node');

// Main request
assert.equal(chains.length, 4, 'generates correct number of chain nodes');
assert.equal(chains[0].querySelector('.crc-node__tree-hostname').textContent, '(example.com)');
assert.ok(!chains[0].querySelector('.lh-text__url-host'), 'should be no origin for root url');
assert.equal(chains[0].querySelector('.lh-text__url a').textContent, 'https://example.com');
assert.equal(chains[0].querySelector('.lh-text__url a').href, 'https://example.com/');
assert.equal(chains[0].querySelector('.lh-text__url a').rel, 'noopener');
assert.equal(chains[0].querySelector('.lh-text__url a').target, '_blank');

// Children
assert.ok(chains[1].querySelector('.crc-node__tree-marker .vert-right'));
assert.equal(chains[1].querySelectorAll('.crc-node__tree-marker .right').length, 2);
assert.equal(chains[1].querySelector('.crc-node__tree-file').textContent, '/b.js');
assert.equal(chains[1].querySelector('.crc-node__tree-hostname').textContent, '(example.com)');
assert.equal(chains[1].querySelector('.lh-text__url a').textContent, '/b.js');
assert.equal(chains[1].querySelector('.lh-text__url a').href, 'https://example.com/b.js');
assert.equal(chains[1].querySelector('.lh-text__url a').rel, 'noopener');
assert.equal(chains[1].querySelector('.lh-text__url a').target, '_blank');
assert.equal(chains[1].querySelector('.lh-text__url-host').textContent, '(example.com)');
const durationNodes = chains[1].querySelectorAll('.crc-node__chain-duration');
assert.equal(durationNodes[0].textContent, ' - 5,000\xa0ms, ');
// Note: actual transferSize is 2000 bytes but formatter formats to KBs.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,10 @@ describe('DetailsRenderer', () => {
assert.equal(urlEl.localName, 'div');
assert.equal(urlEl.title, urlText);
assert.equal(urlEl.dataset.url, urlText);
assert.ok(urlEl.firstChild.classList.contains('lh-text'));
assert.equal(urlEl.firstChild.nodeName, 'A');
assert.equal(urlEl.firstChild.href, urlText);
assert.equal(urlEl.firstChild.rel, 'noopener');
assert.equal(urlEl.firstChild.target, '_blank');
assert.equal(urlEl.textContent, displayUrlText);
});

Expand All @@ -430,7 +433,7 @@ describe('DetailsRenderer', () => {
assert.equal(urlEl.localName, 'div');
assert.equal(urlEl.title, urlText);
assert.equal(urlEl.dataset.url, urlText);
assert.ok(urlEl.firstChild.classList.contains('lh-text'));
assert.equal(urlEl.firstChild.nodeName, 'A');
assert.equal(urlEl.textContent, displayUrlText);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ describe('ReportUIFeatures', () => {

function getUrlsInTable() {
return dom
.findAll('#uses-webp-images .lh-details .lh-text__url .lh-text:first-child', container)
.findAll('#uses-webp-images .lh-details .lh-text__url a:first-child', container)
.map(el => el.textContent);
}

Expand Down

0 comments on commit d9e012b

Please sign in to comment.