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(redesign): add sticky scores header #8524

Merged
merged 23 commits into from
Apr 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 49 additions & 23 deletions lighthouse-core/report/html/renderer/report-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,39 @@ class ReportRenderer {
return container;
}

/**
* @param {LH.ReportResult} report
* @param {CategoryRenderer} categoryRenderer
* @param {Record<string, CategoryRenderer>} specificCategoryRenderers
Copy link
Member

Choose a reason for hiding this comment

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

categoryRenderer and specificCategoryRenderers probably need docstrings (sorry for that second name :)

* @return {DocumentFragment[]}
*/
_renderScoreGauges(report, categoryRenderer, specificCategoryRenderers) {
// Group gauges in this order: default, pwa, plugins.
const defaultGauges = [];
const customGauges = []; // PWA.
const pluginGauges = [];

for (const category of report.reportCategories) {
const renderer = specificCategoryRenderers[category.id] || categoryRenderer;
const categoryGauge = renderer.renderScoreGauge(category, report.categoryGroups || {});

if (Util.isPluginCategory(category.id)) {
pluginGauges.push(categoryGauge);
} else if (renderer.renderScoreGauge === categoryRenderer.renderScoreGauge) {
// The renderer for default categories is just the default CategoryRenderer.
// If the functions are equal, then renderer is an instance of CategoryRenderer.
// For example, the PWA category uses PwaCategoryRenderer, which overrides
// CategoryRenderer.renderScoreGauge, so it would fail this check and be placed
// in the customGauges bucket.
defaultGauges.push(categoryGauge);
} else {
customGauges.push(categoryGauge);
}
}

return [...defaultGauges, ...customGauges, ...pluginGauges];
}

/**
* @param {LH.ReportResult} report
* @return {DocumentFragment}
Expand Down Expand Up @@ -223,29 +256,9 @@ class ReportRenderer {
// }

if (scoreHeader) {
// Group gauges in this order: default, pwa, plugins.
const defaultGauges = [];
const customGauges = []; // PWA.
const pluginGauges = [];
for (const category of report.reportCategories) {
const renderer = specificCategoryRenderers[category.id] || categoryRenderer;
const categoryGauge = renderer.renderScoreGauge(category, report.categoryGroups || {});

if (Util.isPluginCategory(category.id)) {
pluginGauges.push(categoryGauge);
} else if (renderer.renderScoreGauge === categoryRenderer.renderScoreGauge) {
// The renderer for default categories is just the default CategoryRenderer.
// If the functions are equal, then renderer is an instance of CategoryRenderer.
// For example, the PWA category uses PwaCategoryRenderer, which overrides
// CategoryRenderer.renderScoreGauge, so it would fail this check and be placed
// in the customGauges bucket.
defaultGauges.push(categoryGauge);
} else {
customGauges.push(categoryGauge);
}
}
scoreHeader.append(...defaultGauges, ...customGauges, ...pluginGauges);

const scoreGauges =
this._renderScoreGauges(report, categoryRenderer, specificCategoryRenderers);
scoreHeader.append(...scoreGauges);
const scoreScale = this._dom.cloneTemplate('#tmpl-lh-scorescale', this._templateContext);
const scoresContainer = this._dom.find('.lh-scores-container', headerContainer);
scoresContainer.appendChild(scoreHeader);
Expand All @@ -255,10 +268,23 @@ class ReportRenderer {
reportSection.appendChild(this._renderReportFooter(report));

const reportFragment = this._dom.createFragment();

if (!this._dom.isDevTools()) {
const topbarDocumentFragment = this._renderReportTopbar(report);
reportFragment.appendChild(topbarDocumentFragment);
}

if (scoreHeader && !this._dom.isDevTools()) {
const stickyHeader = this._dom.createElement('div', 'lh-sticky-header');
Copy link
Member

Choose a reason for hiding this comment

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

we need a test for this, though jsdom (AFAIK) doesn't support scrolling, so it will have to be a limited one. Since we have a nice complete "renders score gauges in this order" test, maybe just a test that .lh-sticky-header has child gauges that match the ones in .lh-scores-header?

Or something like that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

a test that .lh-sticky-header has child gauges that match the ones in .lh-scores-header?

imo that doesn't seem like a useful test, since these gauges are created with the same function.

jsdom (AFAIK) doesn't support scrolling

jsdom/jsdom#1422 (comment) this makes me think it should be possible to fake it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok it'd be way too much faking, basically all the getBoundingClientRect needs to be mocked

this._dom.createChildOf(stickyHeader, 'div', 'lh-highlighter');

const scoreGauges =
this._renderScoreGauges(report, categoryRenderer, specificCategoryRenderers);
stickyHeader.append(...scoreGauges);

reportFragment.appendChild(stickyHeader);
}

reportFragment.appendChild(headerContainer);
reportFragment.appendChild(container);

Expand Down
43 changes: 43 additions & 0 deletions lighthouse-core/report/html/renderer/report-ui-features.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ class ReportUIFeatures {
this._copyAttempt = false;
/** @type {HTMLElement} */
this.exportButton; // eslint-disable-line no-unused-expressions
/** @type {HTMLElement} */
this.topbarEl; // eslint-disable-line no-unused-expressions
/** @type {HTMLElement} */
this.scoreScaleEl; // eslint-disable-line no-unused-expressions
/** @type {HTMLElement} */
this.stickyHeaderEl; // eslint-disable-line no-unused-expressions
/** @type {HTMLElement} */
this.highlightEl; // eslint-disable-line no-unused-expressions

this.onMediaQueryChange = this.onMediaQueryChange.bind(this);
this.onCopy = this.onCopy.bind(this);
Expand All @@ -48,6 +56,7 @@ class ReportUIFeatures {
this.onKeyDown = this.onKeyDown.bind(this);
this.printShortCutDetect = this.printShortCutDetect.bind(this);
this.onChevronClick = this.onChevronClick.bind(this);
this._updateStickyHeaderOnScroll = this._updateStickyHeaderOnScroll.bind(this);
}

/**
Expand All @@ -61,10 +70,13 @@ class ReportUIFeatures {
this.json = report;
this._setupMediaQueryListeners();
this._setupExportButton();
this._setupStickyHeaderElements();
this._setUpCollapseDetailsAfterPrinting();
this._resetUIState();
this._document.addEventListener('keydown', this.printShortCutDetect);
this._document.addEventListener('copy', this.onCopy);
this._document.addEventListener('scroll', this._updateStickyHeaderOnScroll);
window.addEventListener('resize', this._updateStickyHeaderOnScroll);
}

/**
Expand Down Expand Up @@ -102,6 +114,13 @@ class ReportUIFeatures {
dropdown.addEventListener('click', this.onExport);
}

_setupStickyHeaderElements() {
this.topbarEl = this._dom.find('.lh-topbar', this._document);
this.scoreScaleEl = this._dom.find('.lh-scorescale', this._document);
this.stickyHeaderEl = this._dom.find('.lh-sticky-header', this._document);
this.highlightEl = this._dom.find('.lh-highlighter', this._document);
}

/**
* Handle copy events.
* @param {ClipboardEvent} e
Expand Down Expand Up @@ -380,6 +399,30 @@ class ReportUIFeatures {
this._document.body.removeChild(a);
setTimeout(_ => URL.revokeObjectURL(href), 500);
}

_updateStickyHeaderOnScroll() {
Copy link
Member

Choose a reason for hiding this comment

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

does this need to be called once on startup?

Copy link
Member

Choose a reason for hiding this comment

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

does this need to be called once on startup?

I guess not since it starts out hidden

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TIL - https://stackoverflow.com/a/34095654

The scroll event does not fire on every load, only when refreshing a page that was scrolled, or when navigating to an anchor directly.

So fresh view (top of page), state should be hidden, and the default is just that. Refresh after scrolling a bit, or click on a link w/ an anchor to the report, and a scroll event fires.

// Show sticky header when the score scale begins to go underneath the topbar.
const topbarBottom = this.topbarEl.getBoundingClientRect().bottom;
const scoreScaleTop = this.scoreScaleEl.getBoundingClientRect().top;
const showStickyHeader = topbarBottom >= scoreScaleTop;

// Highlight mini gauge when section is in view.
Copy link
Member

Choose a reason for hiding this comment

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

early return if showStickyHeader === false?

// In view = the last category that starts above the middle of the window.
const categoryEls = Array.from(this._document.querySelectorAll('.lh-category'));
const categoriesAboveTheMiddle =
categoryEls.filter(el => el.getBoundingClientRect().top - window.innerHeight / 2 < 0);
const highlightIndex =
categoriesAboveTheMiddle.length > 0 ? categoriesAboveTheMiddle.length - 1 : 0;

// Category order matches gauge order in sticky header.
const gaugeWrapperEls = this.stickyHeaderEl.querySelectorAll('.lh-gauge__wrapper');
const gaugeToHighlight = gaugeWrapperEls[highlightIndex];
const offset = gaugeToHighlight.getBoundingClientRect().left + 'px';

// Mutate at end to avoid layout thrashing.
this.stickyHeaderEl.classList.toggle('lh-sticky-header--visible', showStickyHeader);
this.highlightEl.style.left = offset;
}
}

if (typeof module !== 'undefined' && module.exports) {
Expand Down
6 changes: 5 additions & 1 deletion lighthouse-core/report/html/report-styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@
--color-fail: var(--color-red);
--color-pass-secondary: var(--color-green-700);
--color-pass: var(--color-green);
--color-sticky-header-bg: var(--color-white);
--color-highlighter-bg: var(--color-black);

--plugin-icon-url: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg" width="24px" height="24px" viewBox="0 0 24 24" fill="%23757575"><path d="M0 0h24v24H0z" fill="none"/><path d="M20.5 11H19V7c0-1.1-.9-2-2-2h-4V3.5C13 2.12 11.88 1 10.5 1S8 2.12 8 3.5V5H4c-1.1 0-1.99.9-1.99 2v3.8H3.5c1.49 0 2.7 1.21 2.7 2.7s-1.21 2.7-2.7 2.7H2V20c0 1.1.9 2 2 2h3.8v-1.5c0-1.49 1.21-2.7 2.7-2.7 1.49 0 2.7 1.21 2.7 2.7V22H17c1.1 0 2-.9 2-2v-4h1.5c1.38 0 2.5-1.12 2.5-2.5S21.88 11 20.5 11z"/></svg>');
--logo-url: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg" width="192" height="192"><g fill="none" fill-rule="evenodd"><path d="M0 0h192v192H0z"/><path d="M67.705 179.352l2.603-20.82 49.335-16.39 4.652 37.21A87.893 87.893 0 0 1 96 184a87.893 87.893 0 0 1-28.295-4.648zM52.44 172.48C25.894 157.328 8 128.754 8 96 8 47.399 47.399 8 96 8s88 39.399 88 88c0 32.754-17.894 61.328-44.44 76.48L130 96h6V80h-8V48L96 28 64 48v32h-8v16h6l-9.56 76.48zM113.875 96l2.882 23.05-43.318 14.433L78.125 96h35.75zM80 80V56.868l16-10 16 10V80H80z" fill="%23000000"/></g></svg>');
Expand Down Expand Up @@ -835,7 +837,9 @@
}

.lh-scores-header .lh-gauge__wrapper,
.lh-scores-header .lh-gauge--pwa__wrapper {
.lh-scores-header .lh-gauge--pwa__wrapper,
.lh-sticky-header .lh-gauge__wrapper,
.lh-sticky-header .lh-gauge--pwa__wrapper {
width: var(--score-container-width);
}

Expand Down
45 changes: 42 additions & 3 deletions lighthouse-core/report/html/templates.html
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,49 @@
position: relative;
width: 100%;
}

.lh-sticky-header {
--gauge-circle-size: 36px;
--plugin-badge-size: 18px;
--plugin-icon-size: 75%;
--score-container-width: 60px;
--score-number-font-size: 13px;
position: fixed;
left: 0;
right: 0;
top: var(--topbar-height);
font-weight: 700;
display: flex;
justify-content: center;
background-color: var(--color-sticky-header-bg);
border-bottom: 1px solid var(--color-black-200);
padding-top: var(--score-container-padding);
padding-bottom: 4px;
z-index: 1000;
pointer-events: none;
opacity: 0;
}

.lh-sticky-header--visible {
pointer-events: auto;
opacity: 1;
patrickhulce marked this conversation as resolved.
Show resolved Hide resolved
}

.lh-sticky-header .lh-gauge__label {
display: none;
}

.lh-highlighter {
width: var(--score-container-width);
height: 1px;
background: var(--color-highlighter-bg);
position: absolute;
bottom: -1px;
left: 0;
}
</style>
<div class="lh-scores-wrapper">
<div class="lh-scores-container">
</div>
<div class="lh-scores-container"></div>
</div>
</template>

Expand Down Expand Up @@ -513,7 +552,7 @@
content: "";
position: absolute;
right: -6px;
bottom: -2px;
bottom: 5px;
display: block;
z-index: 100;
box-shadow: 0 0 4px rgba(0,0,0,.2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,9 @@ describe('ReportRenderer', () => {
const output = renderer.renderReport(sampleResults, container);
assert.ok(output.querySelector('.lh-header-sticky'), 'has a header');
assert.ok(output.querySelector('.lh-report'), 'has report body');
// 3 sets of gauges - one in sticky header, one in scores header, and one in each section.
assert.equal(output.querySelectorAll('.lh-gauge__wrapper, .lh-gauge--pwa__wrapper').length,
sampleResults.reportCategories.length * 2, 'renders category gauges');
sampleResults.reportCategories.length * 3, 'renders category gauges');
Copy link
Member

Choose a reason for hiding this comment

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

maybe add a comment for where this multiplier comes from

// no fireworks
assert.ok(output.querySelector('.score100') === null, 'has no fireworks treatment');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ describe('ReportUIFeatures', () => {
};
};

global.window = {};
global.window = document.window;
global.window.getComputedStyle = function() {
return {
marginTop: '10px',
Expand Down