Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

report: make urls clickable #9224

Merged
merged 18 commits into from
Jun 29, 2019
Merged
4 changes: 3 additions & 1 deletion lighthouse-core/report/html/renderer/crc-details-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,9 @@ class CriticalRequestChainRenderer {
// Fill in url, host, and request size information.
const {file, hostname} = Util.parseURL(segment.node.request.url);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests are failing b/c of this change. DetailsRenderer.renderTextURL tosses the host part if parsed.file === '/'. Before, the chain renderer always used the results of Util.parseURL w/o post-processing. The chain renderer now looks like this when run on the root url of a site:

image

before:

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The chain renderer now looks like this when run on the root url of a site:

this is an improvement, IMO.

const treevalEl = dom.find('.crc-node__tree-value', chainsEl);
dom.find('.crc-node__tree-file', treevalEl).textContent = `${file}`;
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})` : '';

if (!segment.hasChildren) {
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/report/html/renderer/details-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {Omit<LH.Audit.Details.LinkValue, "type">} details
* @return {Element}
*/
_renderLink(details) {
Expand Down
6 changes: 6 additions & 0 deletions lighthouse-core/report/html/report-styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -1372,6 +1372,12 @@
object-fit: contain;
}

.lh-text__url > a,
.lh-crc .crc-node__tree-value > a {
color: inherit;
text-decoration: none;
}

/* Chevron
https://codepen.io/paulirish/pen/LmzEmK
*/
Expand Down
3 changes: 1 addition & 2 deletions lighthouse-core/report/html/templates.html
Original file line number Diff line number Diff line change
Expand Up @@ -769,9 +769,8 @@

</span>
<span class="crc-node__tree-value">
<span class="crc-node__tree-file"><!-- fill me: node.request.url.file --></span>
<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