Skip to content

Commit

Permalink
report: allow CRC to use renderTextUrl (#9237)
Browse files Browse the repository at this point in the history
  • Loading branch information
paulirish authored and connorjclark committed Jun 20, 2019
1 parent 425d7ec commit 7d93bde
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 38 deletions.
23 changes: 12 additions & 11 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,12 +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);
const fileEl = /** @type {HTMLAnchorElement} */ (dom.find('.crc-node__tree-file', treevalEl));
fileEl.textContent = `${file}`;
fileEl.href = segment.node.request.url;
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 @@ -148,14 +147,15 @@ class CriticalRequestChainRenderer {
* @param {CRCSegment} segment
* @param {Element} elem Parent element.
* @param {LH.Audit.Details.CriticalRequestChain} details
* @param {DetailsRenderer} detRenderer
*/
static buildTree(dom, tmpl, segment, elem, details) {
elem.appendChild(CriticalRequestChainRenderer.createChainNode(dom, tmpl, segment));
static buildTree(dom, tmpl, segment, elem, details, detRenderer) {
elem.appendChild(CriticalRequestChainRenderer.createChainNode(dom, tmpl, segment, detRenderer));
if (segment.node.children) {
for (const key of Object.keys(segment.node.children)) {
const childSegment = CriticalRequestChainRenderer.createSegment(segment.node.children, key,
segment.startTime, segment.transferSize, segment.treeMarkers, segment.isLastChild);
CriticalRequestChainRenderer.buildTree(dom, tmpl, childSegment, elem, details);
CriticalRequestChainRenderer.buildTree(dom, tmpl, childSegment, elem, details, detRenderer);
}
}
}
Expand All @@ -164,9 +164,10 @@ class CriticalRequestChainRenderer {
* @param {DOM} dom
* @param {ParentNode} templateContext
* @param {LH.Audit.Details.CriticalRequestChain} details
* @param {DetailsRenderer} detRenderer
* @return {Element}
*/
static render(dom, templateContext, details) {
static render(dom, templateContext, details, detRenderer) {
const tmpl = dom.cloneTemplate('#tmpl-lh-crc', templateContext);
const containerEl = dom.find('.lh-crc', tmpl);

Expand All @@ -182,7 +183,7 @@ class CriticalRequestChainRenderer {
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);
CriticalRequestChainRenderer.buildTree(dom, tmpl, segment, containerEl, details, detRenderer);
}

return dom.find('.lh-crc-container', tmpl);
Expand Down
8 changes: 4 additions & 4 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 Down Expand Up @@ -211,7 +211,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 @@ -256,7 +256,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
29 changes: 14 additions & 15 deletions lighthouse-core/report/html/report-styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -149,29 +149,29 @@
--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);
--color-black-400: var(--color-black-600);
--color-black-600: var(--color-black-500);

--topbar-bg: var(--color-black);
--plugin-badge-bg: var(--color-black-800);
--env-item-bg: var(--color-black);
--report-secondary-border-color: var(--color-black-200);

--body-background-color: var(--color-black-900);
--body-text-color: var(--color-black-100);
--secondary-text-color: var(--color-black-400);

--plugin-icon-url: var(--plugin-icon-url-dark);

--informative-color: var(--color-blue-200);

--medium-50-gray: #757575;
--medium-75-gray: var(--color-white);

--color-hover: rgba(0, 0, 0, 0.2);

--pwa-fast-reliable-gray-url: var(--pwa-fast-reliable-gray-url-dark);
--pwa-installable-gray-url: var(--pwa-installable-gray-url-dark);
--pwa-optimized-gray-url: var(--pwa-optimized-gray-url-dark);
Expand Down Expand Up @@ -1351,12 +1351,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 @@ -1372,12 +1368,15 @@
object-fit: contain;
}

.lh-text__url > a,
.lh-crc .crc-node__tree-value > a {
.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
9 changes: 4 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,7 @@

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

</span>
</div>
</template>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,13 @@ describe('DetailsRenderer', () => {

// 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.equal(chains[0].querySelector('.lh-text__url-host').textContent, '(example.com)');

// 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-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

0 comments on commit 7d93bde

Please sign in to comment.