From b262d7ca496d70fd09ee007586674e10cb4ff3f6 Mon Sep 17 00:00:00 2001 From: Damani Date: Tue, 12 May 2020 13:00:37 -0400 Subject: [PATCH] Added og:image and alt text checks to linter for Stories (#755) * Added og:image and alt text checks to linter for Stories * Minor fixes to linter checks. Added testing for new checks * Removed unused var * Recommitting for CI build, No code changes * Code cleanup Co-authored-by: Damani Brown --- packages/linter/src/index.ts | 4 + .../linter/src/rules/ImagesHaveAltText.ts | 25 ++ .../src/rules/MetadataIncludesOGImageSrc.ts | 30 ++ packages/linter/tests/local.ts | 30 +- .../local/ImagesHaveAltText-1/source.html | 92 ++++++ .../local/ImagesHaveAltText-2/source.html | 92 ++++++ .../MetadataIncludesOGImageSrc-1/source.html | 290 ++++++++++++++++++ .../MetadataIncludesOGImageSrc-2/source.html | 287 +++++++++++++++++ 8 files changed, 849 insertions(+), 1 deletion(-) create mode 100644 packages/linter/src/rules/ImagesHaveAltText.ts create mode 100644 packages/linter/src/rules/MetadataIncludesOGImageSrc.ts create mode 100644 packages/linter/tests/local/ImagesHaveAltText-1/source.html create mode 100644 packages/linter/tests/local/ImagesHaveAltText-2/source.html create mode 100644 packages/linter/tests/local/MetadataIncludesOGImageSrc-1/source.html create mode 100644 packages/linter/tests/local/MetadataIncludesOGImageSrc-2/source.html diff --git a/packages/linter/src/index.ts b/packages/linter/src/index.ts index 02fbd5fc4..6803566a1 100644 --- a/packages/linter/src/index.ts +++ b/packages/linter/src/index.ts @@ -18,6 +18,8 @@ import { SxgVaryOnAcceptAct } from "./rules/SxgVaryOnAcceptAct"; import { SxgContentNegotiationIsOk } from "./rules/SxgContentNegotiationIsOk"; import { SxgDumpSignedExchangeVerify } from "./rules/SxgDumpSignedExchangeVerify"; import { SxgAmppkgIsForwarded } from "./rules/SxgAmppkgIsForwarded"; +import { MetadataIncludesOGImageSrc } from "./rules/MetadataIncludesOGImageSrc"; +import { ImagesHaveAltText } from "./rules/ImagesHaveAltText"; import { RuleConstructor } from "./rule"; import { isArray } from "util"; @@ -100,6 +102,8 @@ function testsForMode(type: LintMode) { StoryMetadataIsV1, StoryIsMostlyText, StoryMetadataThumbnailsAreOk, + MetadataIncludesOGImageSrc, + ImagesHaveAltText, ]) ); return tests.get(type) || []; diff --git a/packages/linter/src/rules/ImagesHaveAltText.ts b/packages/linter/src/rules/ImagesHaveAltText.ts new file mode 100644 index 000000000..b928c7b42 --- /dev/null +++ b/packages/linter/src/rules/ImagesHaveAltText.ts @@ -0,0 +1,25 @@ +import { Context } from "../index"; +import { Rule } from "../rule"; + +export class ImagesHaveAltText extends Rule { + run({ $ }: Context) { + let imgsWithoutAlt = ""; + + $("amp-img").each(function (i, elem) { + if (!elem.attribs.alt) { + imgsWithoutAlt = imgsWithoutAlt + "- " + elem.attribs.src + "\n"; + } + }); + + return imgsWithoutAlt.length > 0 + ? this.warn(`Missing alt text from images: \n` + imgsWithoutAlt) + : this.pass(); + } + meta() { + return { + url: "https://blog.amp.dev/2020/02/12/seo-for-amp-stories/", + title: "Images contain alt text", + info: "", + }; + } +} diff --git a/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts b/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts new file mode 100644 index 000000000..104660414 --- /dev/null +++ b/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts @@ -0,0 +1,30 @@ +import { Context } from "../index"; +import { Rule } from "../rule"; + +export class MetadataIncludesOGImageSrc extends Rule { + run({ $ }: Context) { + let hasOGImage = false; + + $("meta").each(function (i, elem) { + if ( + elem.attribs.property && + elem.attribs.property.includes("og:image", 0) && + elem.attribs.content + ) { + hasOGImage = true; + return false; + } + }); + + return !hasOGImage + ? this.warn(`Missing og:image property or content source`) + : this.pass(); + } + meta() { + return { + url: "https://blog.amp.dev/2020/02/12/seo-for-amp-stories/", + title: "Metadata includes og:image and src", + info: "", + }; + } +} diff --git a/packages/linter/tests/local.ts b/packages/linter/tests/local.ts index ab826706f..5c9cb337a 100644 --- a/packages/linter/tests/local.ts +++ b/packages/linter/tests/local.ts @@ -4,6 +4,8 @@ import { MetaCharsetIsFirst } from "../src/rules/MetaCharsetIsFirst"; import { RuntimeIsPreloaded } from "../src/rules/RuntimeIsPreloaded"; import { SchemaMetadataIsNews } from "../src/rules/SchemaMetadataIsNews"; import { StoryRuntimeIsV1 } from "../src/rules/StoryRuntimeIsV1"; +import { ImagesHaveAltText } from "../src/rules/ImagesHaveAltText"; +import { MetadataIncludesOGImageSrc } from "../src/rules/MetadataIncludesOGImageSrc"; import { basename } from "path"; import { BookendExists } from "../src/rules/BookendExists"; @@ -102,5 +104,31 @@ assertWarn( runLocalTest(BookendExists, "local/BookendExists-3/source.html") ); +assertPass( + `${MetadataIncludesOGImageSrc.name} - is present`, + runLocalTest( + MetadataIncludesOGImageSrc, + "local/MetadataIncludesOGImageSrc-1/source.html" + ) +); + +assertWarn( + `${MetadataIncludesOGImageSrc.name} - is missing`, + runLocalTest( + MetadataIncludesOGImageSrc, + "local/MetadataIncludesOGImageSrc-2/source.html" + ) +); + +assertPass( + `${ImagesHaveAltText.name} - All have alt text`, + runLocalTest(ImagesHaveAltText, "local/ImagesHaveAltText-1/source.html") +); + +assertWarn( + `${ImagesHaveAltText.name} - At least one is missing alt text`, + runLocalTest(ImagesHaveAltText, "local/ImagesHaveAltText-2/source.html") +); + console.log(`# ${basename(__filename)} - HTML-only tests`); -console.log(`1..16`); +console.log(`1..20`); diff --git a/packages/linter/tests/local/ImagesHaveAltText-1/source.html b/packages/linter/tests/local/ImagesHaveAltText-1/source.html new file mode 100644 index 000000000..ab6efb0a9 --- /dev/null +++ b/packages/linter/tests/local/ImagesHaveAltText-1/source.html @@ -0,0 +1,92 @@ + + + + + + + + + + Through the fence 2020 + + + + + + + + + +

Through The Fence 2020

+
+
+ +
+ + diff --git a/packages/linter/tests/local/ImagesHaveAltText-2/source.html b/packages/linter/tests/local/ImagesHaveAltText-2/source.html new file mode 100644 index 000000000..1b8f752db --- /dev/null +++ b/packages/linter/tests/local/ImagesHaveAltText-2/source.html @@ -0,0 +1,92 @@ + + + + + + + + + + Through the fence 2020 + + + + + + + + + +

Through The Fence 2020

+
+
+ +
+ + diff --git a/packages/linter/tests/local/MetadataIncludesOGImageSrc-1/source.html b/packages/linter/tests/local/MetadataIncludesOGImageSrc-1/source.html new file mode 100644 index 000000000..6fa0afdd8 --- /dev/null +++ b/packages/linter/tests/local/MetadataIncludesOGImageSrc-1/source.html @@ -0,0 +1,290 @@ + + + + + + + + + + Through the fence 2020 + + + + + + + + + + + + + + + + + + + + + + + + + + + +

Through The Fence 2020

+
+
+ + +
+ + diff --git a/packages/linter/tests/local/MetadataIncludesOGImageSrc-2/source.html b/packages/linter/tests/local/MetadataIncludesOGImageSrc-2/source.html new file mode 100644 index 000000000..937b2bed5 --- /dev/null +++ b/packages/linter/tests/local/MetadataIncludesOGImageSrc-2/source.html @@ -0,0 +1,287 @@ + + + + + + + + + + Through the fence 2020 + + + + + + + + + + + + + + + + + + + + + + + + +

Through The Fence 2020

+
+
+ + +
+ +