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

tests(treemap): add test for node coverage shading #12609

Merged
merged 5 commits into from
Jun 3, 2021

Conversation

paulirish
Copy link
Member

followup from #12603 (comment)

i wrote the code that was wrong, so here's my penance.


driveby change of port number... no reason it needs to conflict w/ 10200 and i'm always happy to avoid stopping my servers.

@paulirish paulirish requested a review from a team as a code owner June 2, 2021 23:37
@paulirish paulirish requested review from adamraine and removed request for a team June 2, 2021 23:37
@google-cla google-cla bot added the cla: yes label Jun 2, 2021
lighthouse-treemap/test/treemap-test-pptr.js Outdated Show resolved Hide resolved
lighthouse-treemap/test/treemap-test-pptr.js Outdated Show resolved Hide resolved
lighthouse-treemap/test/treemap-test-pptr.js Outdated Show resolved Hide resolved
lighthouse-treemap/test/treemap-test-pptr.js Outdated Show resolved Hide resolved

// Determine visual red shading percentage
const percentRed = await gtmElemHandle.evaluate(node => {
const redWidthPx = parseInt(window.getComputedStyle(node, ':before').width);
Copy link
Collaborator

Choose a reason for hiding this comment

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

would const redWidthPx = node.clientWidth be the same?

oh, no, because pseudoelement. and no way to get a reference to that in JS.

TIL the second parameter of getComputedStyle. 0 : would make sense. 2 does too. but 1?!?

Copy link
Member Author

Choose a reason for hiding this comment

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

would const redWidthPx = node.clientWidth be the same?
oh, no, because pseudoelement. and no way to get a reference to that in JS.

yup exactly.

TIL the second parameter of getComputedStyle. 0 : would make sense. 2 does too. but 1?!?

yeah it's weird. spec says 1 or two colons is fine. https://www.w3.org/TR/cssom-1/#dom-window-getcomputedstyle

looks like chrome works with zero, one or two colons. handy.

Co-authored-by: Connor Clark <cjamcl@google.com>
lighthouse-treemap/test/treemap-test-pptr.js Outdated Show resolved Hide resolved
lighthouse-treemap/test/treemap-test-pptr.js Outdated Show resolved Hide resolved

// Determine visual red shading percentage
const percentRed = await gtmElemHandle.evaluate(node => {
const redWidthPx = parseInt(window.getComputedStyle(node, ':before').width);
Copy link
Member Author

Choose a reason for hiding this comment

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

would const redWidthPx = node.clientWidth be the same?
oh, no, because pseudoelement. and no way to get a reference to that in JS.

yup exactly.

TIL the second parameter of getComputedStyle. 0 : would make sense. 2 does too. but 1?!?

yeah it's weird. spec says 1 or two colons is fine. https://www.w3.org/TR/cssom-1/#dom-window-getcomputedstyle

looks like chrome works with zero, one or two colons. handy.

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