report: introduce the new report api, add dom.rootEl - #13277
Conversation
…esnt scope well already. about to revert
…d, it doesnt scope well already. about to revert" This reverts commit 7779f25.
| */ | ||
| export function renderReport(lhr, opts = {}) { | ||
| const rootEl = document.createElement('main'); | ||
| rootEl.classList.add('lh-root', 'lh-vars'); |
There was a problem hiding this comment.
The standalone template HTML already includes these classes in a main tag. WDYT about making rootEl a normal div, and removing the classes from the template files?
Standalone renderer would still look for the main tag.
There was a problem hiding this comment.
basically did the same kinda thing. just avoiding a double-main scenario. :)
| const topbarDocumentFragment = this._renderReportTopbar(report); | ||
|
|
||
| reportFragment.appendChild(topbarDocumentFragment); | ||
| if (!this._opts.omitTopbar) { |
There was a problem hiding this comment.
The topbar is removed separately in report.tsx. Can we replace that logic with this new option now?
| } | ||
|
|
||
| /** | ||
| * ONLY use if `dom.rootEl` isn't sufficient for your needs. It is preferred for all scoping, |
There was a problem hiding this comment.
Got a little confused on first read
| * ONLY use if `dom.rootEl` isn't sufficient for your needs. It is preferred for all scoping, | |
| * ONLY use if `dom.rootEl` isn't sufficient for your needs. `dom.rootEl` is preferred for all scoping, |
| <body> | ||
|
|
||
| <div class="drop_zone"></div> | ||
| <div class="renderer-container"></div> |
There was a problem hiding this comment.
FWIW my suggestion was to keep this as a main tag and have generated report contents as divs. Generating main in renderReport might create nested mains in the flow report.
| <noscript>Lighthouse report requires JavaScript. Please enable.</noscript> | ||
|
|
||
| <main><!-- report populated here --></main> | ||
| <main class="flow-vars lh-root lh-vars"><!-- report populated here --></main> |
There was a problem hiding this comment.
Todo for me to add these programmatically in the flow report.
| <meta name="viewport" content="width=device-width, initial-scale=1, minimum-scale=1"> | ||
| <link rel="icon" href="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAYAAABzenr0AAAEhklEQVR4AWJxL/BhIAesev1U5tcflpncgNrKIsqNIwzC9feMpDUzs70kOczMzMzJJcxwCTMzncPMnOwtzBwzMzPb0vRfeZPp0VhPS5I39V5fdiXV1/VD+9QC7OVn9BsyH1XIoEI1PfmJvLFowVV564+34DFUHudbmfDh4kVXh//7XwE+WjS/YfXZe3yr4j2rqj1AIhSB7hZ8ZtPZu/zw8cK523U4wE1/rvPfWrz4zs0m9ZdC9yUJAlASdBAgocRegfF/f3/h/PuaFsxMdwjAR0vm1+06eMMfIrhLqTWqdH4EumU2SPfMhigJAlRQbZrgrRsl9U+Y2DYDFCz3ILC9kiAiqSrMwbWT0nceEnR+9Kggc2zjOJCASDENkg0a5HfZZgDP81CM3CrQs2Z1+o7DJ6ePr8sK0AOCHv5Jjdt3evyYSaZ351VIStIxPRAUtrBYbxC6w+BZ0ivVSBKkIhJhemSyZpfB00EiPO2VjzYkxhcqXQqCWCShGplvi3y0QxqbuBurMjyJeWnkHZuAEgIQGsUBqwrfjZ+IlBgKyRJzVVYF8O6qFWdh86YzQzMrZigYmxAyfvHgLZQ/LC1CbeniW2Hkqr/PH16SgvGuf2/uzNMBwJA/njxizGPtSyAf7EziJCMGRDRdhoAC4PL1A/SrKQMAAQkEfpJAcRQdrBJ7gNwjSpJsdwK+CANBkqa1LgQB4IicV9nYUct7gaxuDJUErQIiEAiMxLVOFlKzIktPpT0ggpdpC/8YAHnxbgkUY4tAAFSR7AAXNyAAWHJrA/kHGjzg5nleuwFO7Nd/IoDw4Pm58+4jNLmYG0wRA5bErc2Mr3Y+dXTDW1VvwqbJkzMCHQ4S1GTCBOIgUHJrGdEwqzR+jAp/o2qAZelUDoQnruEEdDclJI6576AlNVfc+22XN/+Y1vnJD0Yind6UpEEvn/Hqq15EYjCW7jZCJEpnNvDgkyelDjs106kuux2AAXCSobULOWP8mLhYlpoDMK4qAFXJGk+grtH8YXVz5KJblqaG1+VUdTc0I290bmUQAriGITRbdQnom0aoFj8kx1+wMD2ifncAXUQE4SkDqN1hE0jEophs1SUwZAOhUAiMCLwRtamtTZtbbmZErSAUHbSysaoEmnrsakiMiUAURi283gN6wans9oX8rOCrj7/JP35DFD+iQ7Au/K2KE1jzx6ujjUnXFH9KjEq6ZlhsTBICrNLJf47Pv/pkHzvup1w4dmUbEei0+bcXRqJuh5kVARQ8byyYxOwNGr7A87xh1tp8sGT+uMInrwi++Xj7TQz2d27NvwEkrOflAFQGIDA5khASBCGdO2/Z/MnLPwYfv5TFhjW7QhVKAB6afwe2LpFlFsCnlQEosgQgDsdOG1/LKeNqJS4JCSPJ/i+TakwEARor7gER1Iva5JmPOJK0RUqmoPnnlzFCtmIAhAAQEIQRgDaiYPIauNXcnDlRIrWNFY3hm7PG9YRqr7IV7HrCgAC17befjEvRq2nGhAHtBqDpOuI/I1diUUAMYIxEdyejBJqLnNoszGZtfiX/CztGv2mq+sdaAAAAAElFTkSuQmCC"> | ||
| <title>Lighthouse Report</title> | ||
| <style>body {margin: 0}</style> |
There was a problem hiding this comment.
Can you add this to the flow template as well, or maybe put in shared styles somehow?
There was a problem hiding this comment.
Hmm looks like the report is rendering in the flow report
There was a problem hiding this comment.
Cool, thanks for fixing those!
following #12772, this PR adds a nice and minimal API for rendering the report. since earlier discussion the report API proposal has even shrank. We no longer need the
renderPartials()approach we previously discussed.In this PR:
dom.rootElas the root DOM element of this report. Critical to have this for our PSI-like case of multiple reports in the same document. You'll spot a bunch of changes from_documenttorootElin report-ui-features. All of those cases would otherwise be bugs.renderReport(lhr, opts)API.omitTopbaranddisableAutoDarkModeAndFireworksoptions.onDetailsItemRendered,onSaveGistandgetStandaloneReportHTMLare not yet here, but those are next.