Refactor report dom, report ui features, etc.

[connorjclark]

lh-root, report-container, main, oh my!

see #12201 (comment)

[paulirish]

Been spending time on this stuff, so i'll brain-dump here..

first, report-ui-features has two distinct sets of functionality: stuff that's related to the topbar, and stuff that isnt.

non-topbar functionality in report-ui-features

• elem screenshots (add css var and install overlay)
• adding lh-narrow. (TODO: remove this unused class)
• third-party filter
• open tab with data (viewer, treemap)
• enable fireworks
• dark mode
• add Button (for treemap or trace)
• show perf metric descriptions when there's a perf metric error

topbar-related functionality in report-ui-features

• sticky-header: setup and onscroll
• topbar dropdown, incl [data-i18n]
• copy LHR to clipboard
• save json/html
• print & collapse

[paulirish]

And more relevant to that linked comment, here's 3 of our report clients' DOM structures:

Standalone report

.lh-root.lh-vars
    <main>
        .lh-topbar
        .lh-container
            (cats.length > 1) ? undefined : .lh-sticky-header
            (cats.length > 1) ? <div> : .lh-header--solo-category
                .lh-header-container
            .lh-report
                .lh-categories
                .lh-footer
            .lh-element-screenshot__overlay

DevTools report

.lh-root.lh-vars
    .lh-topbar
    .lh-container
        (same .lh-container contents as standalone)
    .lh-element-screenshot__overlay (one level higher than standalone)

PSI report (which has two LH reports)

.lh-vars (explicit .element-screenshots-container way up here so the overlay isn't behind PSI's blue sticky header)
    .lh-element-screenshot__overlay
.result-tabs
.result-container
    .result.goog-control.lh-vars.lh-root
        .psi-category-wrapper
            .report-summary
                .lh-score__gauge
                .inspected-url-text
                .lh-scorescale
        .result-body
            .report-body
                .field-data
                .psi-category-wrapper
                    .lab-data
                        .lh-category
    .result.goog-control.lh-vars.lh-root (the Desktop one)
            (the the same stuff as the last .result)

A few things I noticed:

• lh-vars and lh-root serve a very similar role and can probably merge. And there's no lh-report in PSI, but I have an alternate proposal for that.
• PSI doesnt currently render toplevel warnings
• PSI's DOM is the worst, because legacy. it'll be nice to clean this up going forward.
• Since I've been figuring out the elem-screenshot stuff a lot.. I think we probably could tweak some things (what node the overlay defaults to, and where the css vars are installed). But right now I think I'd only move them for aesthetic reasons.
• i saw a few cases where elements are called headerContainer and elemScreenshotContainer, but they're actually not the elements that have those classNames.
• there's probably some room to adjust the core DOM structure (so header/body/footer are siblings under a clear parent container). But at the same time, I don't this part is too critical.

[paulirish]

Current thinking on a new DOM structure....

.lh-vars /* .lh-root merges into that. */
    .lh-topbar
    .lh-main /* rename from lh-container, basically */
        .lh-header /* contains sticky-header and header-container (which both have 5 gauges, typically) */
        .lh-categories
        .lh-footer

Alternatively, we remove this lh-main container entirely, and its children slide up a level. That's also doable and aligns better with exposing these at a component level.

[patrickhulce]

Alternatively, we remove this lh-main container entirely, and its children slide up a level. That's also doable and aligns better with exposing these at a component level.

SGTM 👍

[connorjclark]

Aren't a number of these elements/classes (mostly I am thinking of .lh-main) necessary for layout purposes?

[patrickhulce]

Aren't a number of these elements/classes (mostly I am thinking of .lh-main) necessary for layout purposes?

To me the idea of providing access to the components individually means an embedder can choose to organize them however they desire for layout purposes. In the root report we can choose to have a wrapper div if we really want one, but others shouldn't have to care about it :)

[paulirish]

Aren't a number of these elements/classes (mostly I am thinking of .lh-main) necessary for layout purposes?

i also had been assuming that the current .lh-container (aka lh-main now) was important for layout. but it's not. it's only needed in the devtools case to manage scrolling, but tbh those styles should probably be managed on the devtools side instead.

[connorjclark]

Next action: I'll split off the topbar features to topbar.js, which we'll include in the report renderer and activate by default.

[connorjclark]

There's probably something still here worth attempting, but it's not on any of our radars atm. Closing until someone gets inspired again :)