report(redesign): inline display text. bold audit header text #8592
Conversation
| flex: 1; | ||
| } | ||
|
|
||
| /* Prepend display text with separator. But not in Opportunities. */ | ||
| .lh-audit-group:not(.lh-audit-group--load-opportunities) .lh-audit__display-text:not(:empty):before { | ||
| content: '—'; |
There was a problem hiding this comment.
| content: '—'; | |
| content: '\2014'; |
There was a problem hiding this comment.
can this just go in the HTML? Kind of weird to do it with a pseudo-element
There was a problem hiding this comment.
why? dt encoding issues are resolved
There was a problem hiding this comment.
@brendankenny can't just be HTML, need some sort of logic check. makes sense to me to have the display + logic in one place here.
There was a problem hiding this comment.
can't just be HTML, need some sort of logic check. makes sense to me to have the display + logic in one place here.
I don't think it needs to be. Opportunities and the rest of the audits share text with a .lh-audit__display-text class, but they actually come from different templates, #tmpl-lh-opportunity vs #tmpl-lh-audit. If the #tmpl-lh-audit template gets the em dash inline, then it'll only show up there
There was a problem hiding this comment.
Good catch on the template thing. But would still need a length check - and can't make a CSS rule for preceding sibling (pseudo code below).
.lh-audit__separator {
margin: 0 var(--audit-item-gap);
}
/* not actually a thing */
.lh-audit__display-text:not(:empty) - .lh-audit__separator {
display: none;
}
| flex: 1; | ||
| } | ||
|
|
||
| /* Prepend display text with separator. But not in Opportunities. */ | ||
| .lh-audit-group:not(.lh-audit-group--load-opportunities) .lh-audit__display-text:not(:empty):before { |
There was a problem hiding this comment.
while this selector does look crazy, it turns out to be reasonable.. 👍
| flex: 1; | ||
| } | ||
|
|
||
| /* Prepend display text with separator. But not in Opportunities. */ | ||
| .lh-audit-group:not(.lh-audit-group--load-opportunities) .lh-audit__display-text:not(:empty):before { | ||
| content: '—'; |
There was a problem hiding this comment.
per discussion in chat.. let's use whitespace on both sides of the dash.
Assuming we want to use var(--audit-item-gap) the we'll need some margin in here too:
margin-right: var(--audit-item-gap);
|
these things realllllly need a screenshot. No idea what "inline display text" means here :) |
Co-Authored-By: Hoten <cjamcl@gmail.com>
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the ℹ️ Googlers: Go here for more info. |
| flex: 1; | ||
| } | ||
|
|
||
| /* Prepend display text with em dash separator. But not in Opportunities. */ | ||
| .lh-audit-group:not(.lh-audit-group--load-opportunities) .lh-audit__display-text:not(:empty):before { |
There was a problem hiding this comment.
best practices aren't grouped so aren't getting the :before
paulirish
changed the title
#8185