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(flow): report api #13374

Merged
merged 12 commits into from
Nov 18, 2021
Merged

report(flow): report api #13374

merged 12 commits into from
Nov 18, 2021

Conversation

adamraine
Copy link
Member

Use new report api in the flow report and removes ReportRendererContext. We still do some flow-specific stuff (e.g. removing labels from gauges), but it's handled by options on the report API now.

Individual LH reports in the flow will now be initialized with ReportUIFeatures, adding support for the treemap button and 3P filter in flows. Topbar, dark mode, and fireworks are disabled via report api options.

@adamraine adamraine requested a review from a team as a code owner November 15, 2021 20:55
@adamraine adamraine requested review from connorjclark and removed request for a team November 15, 2021 20:55
@google-cla google-cla bot added the cla: yes label Nov 15, 2021
@adamraine adamraine requested a review from paulirish November 15, 2021 20:55
function __initLighthouseFlowReport__() {
// TODO(adamraine): add lh-vars, etc classes programmatically instead of in the HTML template
Copy link
Member Author

Choose a reason for hiding this comment

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

Drive-by resolving this TODO

* @return {DocumentFragment}
*/
function renderCategoryScore(category, options) {
const dom = new DOM(document, document.documentElement);
Copy link
Member Author

Choose a reason for hiding this comment

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

@paulirish we discussed making these globals, but that lead to some jest issues where the globals are persistent across tests. I didn't think it would be a big deal to create new instances on each call.

Another option would be to use globals and expose a _resetGlobals function for testing or something.

Copy link
Member

Choose a reason for hiding this comment

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

yeah i like what you did here. this seems like a great solution for now.

Copy link
Member

@paulirish paulirish left a comment

Choose a reason for hiding this comment

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

a few thoughts. otherwise this lg

* @return {DocumentFragment}
*/
function renderCategoryScore(category, options) {
const dom = new DOM(document, document.documentElement);
Copy link
Member

Choose a reason for hiding this comment

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

yeah i like what you did here. this seems like a great solution for now.

report/renderer/report-renderer.js Outdated Show resolved Hide resolved
report/renderer/category-renderer.js Outdated Show resolved Hide resolved
report/types/report-renderer.d.ts Outdated Show resolved Hide resolved
@adamraine adamraine merged commit d3f338d into master Nov 18, 2021
@adamraine adamraine deleted the flow-new-report-api branch November 18, 2021 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants