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

fix(complex): ignore traversals selector types #964

Merged

Conversation

rdamlenc
Copy link
Contributor

complex selectors should ignore traversals selector types

So #foo .bar ul li a complixity level would be 5 rather than 9

@macbre
Copy link
Owner

macbre commented Jan 16, 2024

Thanks for the fix.

So #foo .bar ul li a complixity level would be 5 rather than 9

Can you add a test case for that, please?

@Jieiku
Copy link

Jieiku commented Feb 3, 2024

Thanks for this fix, really looking forward to it :)

@Jieiku
Copy link

Jieiku commented Feb 24, 2024

@rdamlenc is this ready to merge? I noticed @macbre mentioned adding a test case?

@coveralls
Copy link

coveralls commented Feb 24, 2024

Pull Request Test Coverage Report for Build 8064517022

Details

  • 0 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 8064500565: 0.0%
Covered Lines: 555
Relevant Lines: 555

💛 - Coveralls

@macbre
Copy link
Owner

macbre commented Feb 25, 2024

@rdamlenc is this ready to merge? I noticed @macbre mentioned adding a test case?

I you can provide some selectors examples (with the expected complexity,), I'll be more than happy to add the test coverage here.

@Jieiku
Copy link

Jieiku commented Feb 25, 2024

In my case analyze-css is used by yellowlabtools. YellowLabTools reports complex selectors as being more than 3 selectors, but now it seems to be including selectors that were previously considered OK.

in the old version it reports as expected: https://yellowlab.tools/result/gtr25fqnyn/rule/cssComplexSelectors

but the new version being developed reports very differently: http://yellowlab.tools:8282/result/gtr26h6mbq/rule/cssComplexSelectors

You can see the list of detected selectors in the second link, but a couple from that list include:

These should be returned as 3 selectors complexity

footer nav a
main nav a
main nav span

and in more detail

  footer nav {
    a {
        margin: .2rem;
    }
    i {
      margin-bottom: .2rem;
    }
  }

  main nav {
    margin-top: var(--s2);//pagination margin
    a,span {
      margin: .2rem;
    }
  }

both are the same site: https://abridge.netlify.app/

This once merged will reportedly fix the issue: YellowLabTools/YellowLabTools#386 (comment)

macbre added a commit that referenced this pull request Feb 27, 2024
@macbre macbre changed the base branch from master to feature/complex-selectors February 27, 2024 16:26
@macbre
Copy link
Owner

macbre commented Feb 27, 2024

I've rebased this PR to merge into c9d6af8 that introduces some test cases.

@macbre macbre merged commit 635a699 into macbre:feature/complex-selectors Feb 28, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants