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.

Sign up for GitHub

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

Jump to bottom

report: fix explanation rendering #9439

Merged
merged 1 commit into from
Jul 24, 2019
Merged

report: fix explanation rendering #9439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions lighthouse-core/audits/seo/font-size.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ const UIStrings = {
/** Explanatory message stating that there was a failure in an audit caused by a missing page viewport meta tag configuration. "viewport" and "meta" are HTML terms and should not be translated. */
explanationViewport: 'Text is illegible because there\'s no viewport meta tag optimized ' +
'for mobile screens.',
/** Explanatory message stating that there was a failure in an audit caused by a certain percentage of the text on the page being too small. "decimalProportion" will be replaced by a percentage between 0 and 100%. */
explanation: '{decimalProportion, number, extendedPercent} of text is too small.',
/** Explanatory message stating that there was a failure in an audit caused by a certain percentage of the text on the page being too small, based on a sample size of text that was less than 100% of the text on the page. "decimalProportion" will be replaced by a percentage between 0 and 100%. */
explanationWithDisclaimer: '{decimalProportion, number, extendedPercent} of text is too ' +
'small (based on {decimalProportionVisited, number, extendedPercent} sample).',
Expand Down Expand Up @@ -328,9 +326,6 @@ class FontSize extends Audit {
decimalProportion: percentageOfFailingText,
decimalProportionVisited: percentageOfVisitedText,
});
} else {
explanation = str_(UIStrings.explanation,
{decimalProportion: percentageOfFailingText});
}
}

Expand Down
4 changes: 0 additions & 4 deletions lighthouse-core/lib/i18n/en-US.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1135,10 +1135,6 @@
"message": "{decimalProportion, number, extendedPercent} legible text",
"description": "Label for the audit identifying font sizes that are too small."
},
"lighthouse-core/audits/seo/font-size.js | explanation": {
"message": "{decimalProportion, number, extendedPercent} of text is too small.",
"description": "Explanatory message stating that there was a failure in an audit caused by a certain percentage of the text on the page being too small. \"decimalProportion\" will be replaced by a percentage between 0 and 100%."
},
"lighthouse-core/audits/seo/font-size.js | explanationViewport": {
"message": "Text is illegible because there's no viewport meta tag optimized for mobile screens.",
"description": "Explanatory message stating that there was a failure in an audit caused by a missing page viewport meta tag configuration. \"viewport\" and \"meta\" are HTML terms and should not be translated."
Expand Down
3 changes: 0 additions & 3 deletions lighthouse-core/lib/i18n/locales/en-XL.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -851,9 +851,6 @@
"lighthouse-core/audits/seo/font-size.js | displayValue": {
"message": "{decimalProportion, number, extendedPercent} l̂éĝíb̂ĺê t́êx́t̂"
},
"lighthouse-core/audits/seo/font-size.js | explanation": {
"message": "{decimalProportion, number, extendedPercent} ôf́ t̂éx̂t́ îś t̂óô śm̂ál̂ĺ."
},
"lighthouse-core/audits/seo/font-size.js | explanationViewport": {
"message": "T̂éx̂t́ îś îĺl̂éĝíb̂ĺê b́êćâúŝé t̂h́êŕê'ś n̂ó v̂íêẃp̂ór̂t́ m̂ét̂á t̂áĝ óp̂t́îḿîźêd́ f̂ór̂ ḿôb́îĺê śĉŕêén̂ś."
},
Expand Down
3 changes: 2 additions & 1 deletion lighthouse-core/report/html/report-styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -951,8 +951,9 @@
}

.lh-audit-explanation {
margin: var(--audit-padding-vertical) 0 calc(var(--audit-padding-vertical) / 2);
margin: var(--audit-padding-vertical) 0 calc(var(--audit-padding-vertical) / 2) var(--audit-margin-horizontal);
line-height: var(--audit-explanation-line-height);
display: inline-block;
Copy link
Collaborator

Choose a reason for hiding this comment

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

what practical effect does this have for our purposes?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's currently a div (w/ display block). this forces a newline

i need it to be inline but also with horizontal margin.

i could change it from a div to a span and maybe drop this display prop but w/e

}

.lh-audit--fail .lh-audit-explanation {
Expand Down
1 change: 0 additions & 1 deletion lighthouse-core/test/audits/seo/font-size-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ describe('SEO: Font size audit', () => {

const auditResult = await FontSizeAudit.audit(artifacts, getFakeContext());
assert.equal(auditResult.score, 0);
expect(auditResult.explanation).toBeDisplayString('41% of text is too small.');
expect(auditResult.displayValue).toBeDisplayString('59% legible text');
});

Expand Down