report(third-party-summary): show resources for entity #11219
Conversation
warrengm
changed the title
warrengm
changed the title
|
@patrickhulce FYI since I don't think I have permissions to add you to the PR |
patrickhulce
left a comment
patrickhulce
left a comment
There was a problem hiding this comment.
thanks @warrengm for keeping me posted I think this looks great! :)
posted my thoughts on when to hide the subitems too 👍
|
|
||
| const runningSummary = {transferSize: 0, blockingTime: 0}; | ||
| const minTransfer = Math.max(2000, stats.transferSize / 20); | ||
| const maxSubItems = Math.min(5, items.length); |
There was a problem hiding this comment.
At first I was against the min transfer idea but the thought of filling the table with 5 tracking pixels for every ad network convinced me :) I agree we may not always want to show the subitem breakdown. I'd say we skip when no resources meeting the min thresholds (i.e. don't force always showing 1 resource + "Other").
extract all these constants to a file up top with explanations? I'd say we could up the threshold to 4 KiB 4096 too
There was a problem hiding this comment.
Will make the change. The pixels are definitely what prompted the change. Are you saying to omit the Other row entirely or keep it for large enough resources? I have mixed feelings either way--I don't think there is a perfect way to display all the information with static tables.
As an aside, it might be good to highlight the number of third party pixels somewhere.
There was a problem hiding this comment.
I also considered omitting the breakdown if there is exactly one large resource, but it might be worth keeping since the third party to script mapping might not be obvious to users.
There was a problem hiding this comment.
Yeah I like keeping it for the one large resource case, especially if there's blocking time to identify which specific script is the problem.
Agreed there's no obvious answer here, but I'd err on the side of not showing pixels since the main point of this is supposed to be impact to TBT and if they're just small network requests we don't really want to dedicate screen space to them.
patrickhulce
left a comment
patrickhulce
left a comment
There was a problem hiding this comment.
LGTM thanks @warrengm! 🎉
connorjclark
changed the title
connorjclark
changed the title
connorjclark
changed the title
connorjclark
changed the title
5 participants
Summary
Adds per-script breakdowns to the Third Party Impact audit.
Sample screenshots showing the general case:
Other screenshots showing some edgier cases:
First, a third party with one resource:
Second, a third party with two resources:
Lastly, a third party with one main resource:
Open question: should we ever not show breakdowns? E.g. if the impact is less than 5kb and 0ms main thread time then it may not be worth doing so. It would be nice if tables were collapse too.
Related Issues/PRs
Issue #11165