Skip to content
This repository has been archived by the owner on Aug 10, 2022. It is now read-only.

[lightouse] update template & add "font sizes" reference #5810

Merged
merged 2 commits into from
Feb 22, 2018

Conversation

kaycebasques
Copy link
Contributor

@kaycebasques kaycebasques commented Feb 21, 2018

Hey @rviscomi and @kdzwinel, have a look at the reference for the "font size" audit, plz!

What's changed, or what was fixed?

  • update template for LH reference docs
  • add reference for "font sizes" audit

Fixes: N/A

Target Live Date: 2018-02-28

  • This has been reviewed and approved by @kdzwinel & @rviscomi
  • I have run gulp test locally and all tests pass.
  • I have added the appropriate type-something label.
  • I've staged the site and manually verified that my content displays correctly.

CC: @petele

@WebFundBot
Copy link

👍

Copy link
Member

@rviscomi rviscomi left a comment

Choose a reason for hiding this comment

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

👍


[viewport]: /web/tools/lighthouse/audits/has-viewport-meta-tag

### Characters per line {: #characters }
Copy link
Member

Choose a reason for hiding this comment

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

This guidance isn't directly relevant to the audit so it's ok to drop it. The current "Learn more" link only points here because the PSI docs on font sizes disappeared and redirected here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I get what you're saying. What I was thinking is that it's nice to use this reference as an entry point to related topics that the developer might be interested in, but the tradeoff is that it clutters up the reference with less-relevant content. Something to consider in general.

cc @paulirish & @vinamratasingal

Copy link
Member

@paulirish paulirish Feb 22, 2018

Choose a reason for hiding this comment

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

yeah line length is technically a readability concern and separate from the legibility of small glyphs. 😛

but i think we should let prescriptive docs speak for themselves. and just link em without further elaboration.

@@ -48,6 +48,8 @@ toc:
path: /web/tools/lighthouse/audits/aspect-ratio
- title: Document Does Not Have A Meta Description
path: /web/tools/lighthouse/audits/description
- title: Document Doesn't Use Legible Font Sizes
Copy link
Member

Choose a reason for hiding this comment

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

Should we update the audit config to use "Does Not" for consistency with other audits?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One way or the other, I was thinking about putting a PR in to make it consistent across reports

Copy link
Contributor Author

@kaycebasques kaycebasques Feb 21, 2018

Choose a reason for hiding this comment

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

Contracting it makes it feel a little more informal, which seems more in line with LH's voice

@WebFundBot
Copy link

👍

@kaycebasques kaycebasques merged commit d432f8c into master Feb 22, 2018
@WebFundBot
Copy link

🎉 This has been pushed live to https://developers.google.com/web/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants