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

misc(treemap): shade background for unused bytes #12486

Merged
merged 12 commits into from
May 18, 2021
Merged

Conversation

paulirish
Copy link
Member

@paulirish paulirish commented May 14, 2021

@google-cla google-cla bot added the cla: yes label May 14, 2021
@connorjclark
Copy link
Collaborator

I think it's too busy... prefer this:

image

also concerned how user would know which shade is unsued vs used

also concerned that we'd have two different ways to visualizing coverage.

would like to see: only highlighed nodes (the most egregious unused nodes) getting a special treatment; and that treatment being the same thing used in the coverage table column

@adamraine
Copy link
Member

also concerned how user would know which shade is unsued vs used

I agree, couldn't tell which shade represents "used" at first glance.

@paulirish
Copy link
Member Author

too busy

yes. it's def busy.

i prefer this:

yeah the thing i don't like about that is the arbitrary thresholds. I think there are likely still modules of interest that are slightly below those numbers. it's hard to hit a binary threshold right so i want to make it a continuous thing.

I agree, couldn't tell which shade represents "used" at first glance.

yeah totally fair. i'm gonna try a candystripe shading to see if it helps.

@connorjclark
Copy link
Collaborator

+1 to the new changes Paul demoed in today's meeting.

@connorjclark connorjclark changed the title treemap: bicolor background for unused % misc(treemap): shade background for unused bytes May 17, 2021
@paulirish paulirish marked this pull request as ready for review May 17, 2021 23:48
@paulirish paulirish requested a review from a team as a code owner May 17, 2021 23:48
@paulirish paulirish requested review from connorjclark and removed request for a team May 17, 2021 23:48
lighthouse-treemap/app/src/main.js Outdated Show resolved Hide resolved
lighthouse-treemap/app/src/main.js Outdated Show resolved Hide resolved
@@ -201,6 +208,27 @@ header {
outline: 2px solid black;
}

.webtreemap-node::before {
Copy link
Collaborator

Choose a reason for hiding this comment

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

magic? magic. ok.

lighthouse-treemap/app/styles/treemap.css Outdated Show resolved Hide resolved
@connorjclark connorjclark mentioned this pull request May 18, 2021
24 tasks
Co-authored-by: Connor Clark <cjamcl@google.com>
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