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

report: fix zebra styling, sub item rows for 3p filter #10978

Merged
merged 4 commits into from
Jun 17, 2020

Conversation

connorjclark
Copy link
Collaborator

#10867 broke the 3p filter in two significant ways: zebra styling didn't take into account the CSS class approach we now have, and sub item rows weren't filtered w/ their 3p parent row.

@connorjclark connorjclark requested a review from a team as a code owner June 17, 2020 00:38
@connorjclark connorjclark requested review from patrickhulce and removed request for a team June 17, 2020 00:38
@patrickhulce
Copy link
Collaborator

we should get something into the sample JSON vercel deployment that has subitems. should we add experimental audits to our sample JSON generation?

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

nice find thanks @connorjclark! mostly the test request :)

@@ -191,7 +191,8 @@ describe('ReportUIFeatures', () => {

function getUrlsInTable() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we get a test for the subitem filtering in sync with their parent row? :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

const shouldHideThirdParty = e.target instanceof HTMLInputElement && !e.target.checked;
let even = true;
let rowEl = rowEls[0];
while (rowEl) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this took a minute for me to parse with the double nested while, I don't have stellar suggestions though. I think making the innermost loop as simple as possible works for me :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

nice 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants