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

core(lazy-lcp): fix failureTitle in lcp-lazy-loaded #13049

Merged
merged 6 commits into from
Sep 14, 2021

Conversation

patrickhulce
Copy link
Collaborator

Summary
The failureTitle was defined in this audit but not actually exported on the meta doh! This means the error message that the user receives is written backwards 😞

We had validation for this to prevent such cases, but it only applied when the scoreDisplayMode was explicit, so this PR also fixes our validation to fail if the default scoreDisplayMode of binary is used.

Related Issues/PRs
Fixes #13048

@patrickhulce
Copy link
Collaborator Author

@adamraine would you mind giving my descriptions/new titles one more pass? tests caught a few more missing titles

@@ -17,6 +17,10 @@ const URL = require('../../lib/url-shim.js');
const i18n = require('../../lib/i18n/i18n.js');

const UIStrings = {
/** Descriptive title of a Lighthouse audit that checks if images match their displayed dimensions. This is displayed when the audit is passing. */
title: 'Images were appropriate for their displayed size',
Copy link
Member

Choose a reason for hiding this comment

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

It's not ideal to duplicate the title, but it's the best way to get around this new check. Could we do a quick unit test to enforce these titles are the same?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh they're explicitly not the same because one is an opportunity which uses imperative titles and this one is a descriptive title that has the binary flip.

@adamraine

This comment has been minimized.

@adamraine
Copy link
Member

Turns out I forgot to do a yarn build-report

@patrickhulce patrickhulce merged commit 9e22b66 into master Sep 14, 2021
@patrickhulce patrickhulce deleted the patrickhulce-patch-1 branch September 14, 2021 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lighthouse 8.4.0 reports LCP image not lazily loaded, yet it has loading="lazy"
3 participants