report: snapshot/timespan sticky header #13617
Conversation
adamraine
changed the title
|
|
||
| // Mutate at end to avoid layout thrashing. | ||
| this.highlightEl.style.transform = `translate(${offset}px)`; |
There was a problem hiding this comment.
The above comment applied to this line. Is there not layout thrashing with these changes? I guess not because you made CSS changes to replace the manual transform setting?
There was a problem hiding this comment.
Can you confirm w/ the performance panel that scrolling doesn't result in tons of style recalc?
There was a problem hiding this comment.
There is some small style updates, but those existed before this change.
There was a problem hiding this comment.
Paul reminded me... before, this highlighter was animated between these states. Now that's lost.
idk if we care tbh
There was a problem hiding this comment.
also we noticed that clicking the gauges at the top doesnt always update highlighter correctly, viewport size seems to make it vary
There was a problem hiding this comment.
The highlight is based on whichever category is in the center of the viewport. For snapshot, performance category can be so small that it will never be in the center.
Honestly, one solution could be to make Accessibility the first category for snapshot reports. Performance isn't important enough to be first for snapshot reports IMO.
Individual snapshot/timespan reports were emitting a bunch of errors because of the sticky header. I also added category labels to the header.
Sample
Mocks: Go to UXR prototype on the sidebar
#11313