report: add treemap button, refactor icon styles#12392
Conversation
adamraine
left a comment
There was a problem hiding this comment.
We can also use this new icon button in devtools for the "View Trace" button.
Could we get the shape and style of this new button to match "View Trace"? Or are you planning to change "View Trace" down the line when treemap moves into the default config?
I planned to change the view trace button to use this icon button next release. |
paulirish
left a comment
There was a problem hiding this comment.
Could we get the shape and style of this new button to match "View Trace"? Or are you planning to change "View Trace" down the line when treemap moves into the default config?
we could tweak the icon button a bit to match this. wdyt @paulirish
yup lg! i was actually also tweaking the look and my first pass ended up with something quite similar:
|
Updated first post with new screenshots. I refactored |
paulirish
left a comment
There was a problem hiding this comment.
a few small things first, but seems good!
| /** | ||
| * @param {{text: string, icon?: string, onClick: () => void}} opts | ||
| */ | ||
| createButton(opts) { |
There was a problem hiding this comment.
addButton since it adds it to the dom? (from the name i didnt expect it to do the append too, but it makes sense)
| background-image: url('data:image/svg+xml;utf8,<svg xmlns="http://www.w3.org/2000/svg" height="24px" viewBox="0 0 24 24" width="24px" fill="black"><path d="M0 0h24v24H0V0z" fill="none"/><path d="M3 5v14h19V5H3zm2 2h15v4H5V7zm0 10v-4h4v4H5zm6 0v-4h9v4h-9z"/></svg>'); | ||
| } | ||
|
|
||
| .lh-button { |
There was a problem hiding this comment.
the component approach of the reusable report-icon in both dropdown items and lh-button makes sense after you think about it for a bit.
so.
yeah sure. cool. 👍
| .lh-tools__button.active + .lh-tools__dropdown { | ||
| opacity: 1; | ||
| clip: rect(-1px, 187px, 242px, -3px); | ||
| clip: rect(-1px, 194px, 242px, -3px); |
There was a problem hiding this comment.
this clip is so weird. whyd we do it? ooh for a composited animation. geez. we're funny.
| .dark .lh-tools__dropdown a:focus { | ||
| background-color: #BDBDBD; | ||
| } | ||
| /* copy icon needs slight adjustments to look great */ |
There was a problem hiding this comment.
this selector doesnt apply anymore fwiw.
it seemed weird so i checked it out. it seems to be shrinking the icon to not be so big.
IMO its not a huge deal so we can just nuke the rule. (or move it to .report-icon--copy::before)
Only shows if there is treemap data present.
We can also use this new icon button in devtools for the "View Trace" button.
ref #11254
tool menu before
second pass
first pass
hover