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

Use universally correct CSS specificity #12001

Closed
istarkov opened this issue Jan 26, 2021 · 6 comments · Fixed by #13501
Closed

Use universally correct CSS specificity #12001

istarkov opened this issue Jan 26, 2021 · 6 comments · Fixed by #13501
Assignees
Labels

Comments

@istarkov
Copy link

istarkov commented Jan 26, 2021

Modern time we have css grids,
and grids allows to create same aspect ratio cells using different methods and without wrapping image with aspect ratio div.

i.e. https://codepen.io/istarkov/pen/RwGXVYp

<div class="relative">
  <div id="aspect-ratio-setter" style="width: 25%" class="bg-red-200">
    <div style="padding-top: 57%;"></div>
  </div>
  <div
    class="grid gap-4 absolute inset-0"
    style="grid-template-columns: repeat(200, 25%)"
  >
    <img class="block w-full h-full" src="https://via.placeholder.com/300/09f/fff.png"/>
    <img class="block w-full h-full" src="https://via.placeholder.com/300/09f/fff.png"/> 
  </div>
</div>

See codepen images has ratio I set in #aspect-ratio-setter

Solution above allows to create carousels, image boards, etc with 2x less (or even 3x if we calc pseudo) elements.

And above is not the only solution with grids I know and sometimes use others.

Can we not show this diagnostic warning if Cumulative Layout Shift is equal to zero?

PS: I cant set width and height attrs on image element as we use responsive aspect ratio.

@patrickhulce
Copy link
Collaborator

Thanks for filing @istarkov! The situation you describe is fine and isn't flagged by the audit (any explicit width/height whether via attributes or CSS passes). I'm not able to reproduce an audit failure on that codepen either.

image

@istarkov
Copy link
Author

Not a failure we have
image

RED TRIANGLE 😱

And even the text above says:
DiagnosticsMore information about the performance of your application. These numbers don't directly affect the Performance score.
Every time I feel that if not directly but somehow it affects our score.

Probably without the red triangle there will be no issues.

Also running my codepen as standalone page https://cdpn.io/istarkov/debug/RwGXVYp/XBrGRjZKeEOM
I see that red triangle

image

I want the same grey circle as you ;-)

@patrickhulce
Copy link
Collaborator

Thanks for the clarification URL @istarkov I ran the wrong fullpage link, I can reproduce it now :)

@patrickhulce
Copy link
Collaborator

patrickhulce commented Apr 26, 2021

The repro URL no longer has tailwindcss on it so it's not a valid bug, but https://codepen.io/patrickhulce/pen/RwKdyKw still reproduces.

It appears to be that the gatherer is not correctly applying CSS specificity and determining the height is auto instead of 100%, legit bug

@adamraine
Copy link
Member

This has been fixed, but we still aren't perfect with how we measure CSS specificity. Possible approaches discussed:

@adamraine adamraine added P3 and removed P1.5 labels Dec 15, 2021
@adamraine adamraine changed the title Diagnostic "Always include width and height" is wrong User universally correct CSS specificity Dec 15, 2021
@adamraine adamraine changed the title User universally correct CSS specificity Use universally correct CSS specificity Dec 15, 2021
@connorjclark
Copy link
Collaborator

Use the order of rules from DevTools as indication of specificity

#13501

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 a pull request may close this issue.

5 participants