From fa67f68734b1417b50e7f0a55715754326369895 Mon Sep 17 00:00:00 2001 From: Damani Brown Date: Thu, 7 May 2020 13:25:38 -0400 Subject: [PATCH 1/5] Added og:image and alt text checks to linter for Stories --- packages/linter/src/index.ts | 6 +++- .../linter/src/rules/ImagesHaveAltText.ts | 30 +++++++++++++++++++ .../src/rules/MetadataIncludesOGImageSrc.ts | 30 +++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 packages/linter/src/rules/ImagesHaveAltText.ts create mode 100644 packages/linter/src/rules/MetadataIncludesOGImageSrc.ts diff --git a/packages/linter/src/index.ts b/packages/linter/src/index.ts index 02fbd5fc4..fc04e383b 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) || []; @@ -148,4 +152,4 @@ export async function lint( ); } -export { cli }; +export { cli }; \ No newline at end of file diff --git a/packages/linter/src/rules/ImagesHaveAltText.ts b/packages/linter/src/rules/ImagesHaveAltText.ts new file mode 100644 index 000000000..31ff624cd --- /dev/null +++ b/packages/linter/src/rules/ImagesHaveAltText.ts @@ -0,0 +1,30 @@ + +import { Context } from "../index"; +import { Rule } from "../rule"; + +export class ImagesHaveAltText extends Rule { + run({ $ }: Context) { + let containsAltText = true; + let imgsWithoutAlt = ""; + + $('amp-img').each(function (i, elem) { + if(!(elem.attribs.alt)) { + containsAltText = false; + imgsWithoutAlt = imgsWithoutAlt + "- " + elem.attribs.src + "\n"; + } + + }); + + return !containsAltText + ? this.warn(`Missing alt text from images: \n` + imgsWithoutAlt) + : this.pass(); + } + meta() { + return { + url: + "https://html.spec.whatwg.org/multipage/parsing.html#determining-the-character-encoding", + title: "Images contain alt text", + info: "", + }; + } +} \ No newline at end of file diff --git a/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts b/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts new file mode 100644 index 000000000..e8acf3f01 --- /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 === 'og:image' && elem.attribs.content) { + hasOGImage = true; + return false; //break the loop + } + }); + + + return !(hasOGImage) + ? this.warn(`Missing og:image property or content source`) + : this.pass(); + } + meta() { + return { + url: + "https://html.spec.whatwg.org/multipage/parsing.html#determining-the-character-encoding", + title: "Metadata includes og:image and src", + info: "", + }; + } +} + From e8a19b7bdc1bed8fc6602e12e165fd118ac3316f Mon Sep 17 00:00:00 2001 From: Damani Brown Date: Mon, 11 May 2020 16:09:02 -0400 Subject: [PATCH 2/5] Minor fixes to linter checks. Added testing for new checks --- packages/linter/src/index.ts | 2 +- .../linter/src/rules/ImagesHaveAltText.ts | 15 +- .../src/rules/MetadataIncludesOGImageSrc.ts | 19 +- packages/linter/tests/local.ts | 30 +- .../local/ImagesHaveAltText-1/source.html | 290 ++++++++++++++++++ .../local/ImagesHaveAltText-2/source.html | 290 ++++++++++++++++++ .../MetadataIncludesOGImageSrc-1/source.html | 290 ++++++++++++++++++ .../MetadataIncludesOGImageSrc-2/source.html | 287 +++++++++++++++++ 8 files changed, 1202 insertions(+), 21 deletions(-) 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 fc04e383b..6803566a1 100644 --- a/packages/linter/src/index.ts +++ b/packages/linter/src/index.ts @@ -152,4 +152,4 @@ export async function lint( ); } -export { cli }; \ No newline at end of file +export { cli }; diff --git a/packages/linter/src/rules/ImagesHaveAltText.ts b/packages/linter/src/rules/ImagesHaveAltText.ts index 31ff624cd..b928c7b42 100644 --- a/packages/linter/src/rules/ImagesHaveAltText.ts +++ b/packages/linter/src/rules/ImagesHaveAltText.ts @@ -1,30 +1,25 @@ - import { Context } from "../index"; import { Rule } from "../rule"; export class ImagesHaveAltText extends Rule { run({ $ }: Context) { - let containsAltText = true; let imgsWithoutAlt = ""; - $('amp-img').each(function (i, elem) { - if(!(elem.attribs.alt)) { - containsAltText = false; + $("amp-img").each(function (i, elem) { + if (!elem.attribs.alt) { imgsWithoutAlt = imgsWithoutAlt + "- " + elem.attribs.src + "\n"; } - }); - return !containsAltText + return imgsWithoutAlt.length > 0 ? this.warn(`Missing alt text from images: \n` + imgsWithoutAlt) : this.pass(); } meta() { return { - url: - "https://html.spec.whatwg.org/multipage/parsing.html#determining-the-character-encoding", + url: "https://blog.amp.dev/2020/02/12/seo-for-amp-stories/", title: "Images contain alt text", info: "", }; } -} \ No newline at end of file +} diff --git a/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts b/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts index e8acf3f01..e605eab58 100644 --- a/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts +++ b/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts @@ -3,28 +3,29 @@ import { Rule } from "../rule"; export class MetadataIncludesOGImageSrc extends Rule { run({ $ }: Context) { - let hasOGImage = false; + let e: string = ""; - $('meta').each(function (i, elem) { - if(elem.attribs.property === 'og:image' && elem.attribs.content) { + $("meta").each(function (i, elem) { + if ( + elem.attribs.property && + elem.attribs.property.includes("og:image", 0) && + elem.attribs.content + ) { hasOGImage = true; - return false; //break the loop + return false; //break the loop } }); - - return !(hasOGImage) + return !hasOGImage ? this.warn(`Missing og:image property or content source`) : this.pass(); } meta() { return { - url: - "https://html.spec.whatwg.org/multipage/parsing.html#determining-the-character-encoding", + url: "https://ogp.me/", title: "Metadata includes og:image and src", info: "", }; } } - diff --git a/packages/linter/tests/local.ts b/packages/linter/tests/local.ts index ab826706f..31559965a 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 presentt`, + 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..03896941b --- /dev/null +++ b/packages/linter/tests/local/ImagesHaveAltText-1/source.html @@ -0,0 +1,290 @@ + + + + + + + + + + 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..2552a578a --- /dev/null +++ b/packages/linter/tests/local/ImagesHaveAltText-2/source.html @@ -0,0 +1,290 @@ + + + + + + + + + + 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

+
+
+ + +
+ + From e9f7ce10ec15fa520c149dde0dd8e205b59796ac Mon Sep 17 00:00:00 2001 From: Damani Brown Date: Mon, 11 May 2020 16:20:49 -0400 Subject: [PATCH 3/5] Removed unused var --- packages/linter/src/rules/MetadataIncludesOGImageSrc.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts b/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts index e605eab58..476d409a5 100644 --- a/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts +++ b/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts @@ -4,7 +4,6 @@ import { Rule } from "../rule"; export class MetadataIncludesOGImageSrc extends Rule { run({ $ }: Context) { let hasOGImage = false; - let e: string = ""; $("meta").each(function (i, elem) { if ( @@ -13,7 +12,7 @@ export class MetadataIncludesOGImageSrc extends Rule { elem.attribs.content ) { hasOGImage = true; - return false; //break the loop + return false; } }); From b41933a8aebad8e32b26f914e74e628c3b21a063 Mon Sep 17 00:00:00 2001 From: Damani Brown Date: Mon, 11 May 2020 16:29:23 -0400 Subject: [PATCH 4/5] Recommitting for CI build, No code changes --- packages/linter/src/rules/MetadataIncludesOGImageSrc.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts b/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts index 476d409a5..12df29b26 100644 --- a/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts +++ b/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts @@ -12,7 +12,7 @@ export class MetadataIncludesOGImageSrc extends Rule { elem.attribs.content ) { hasOGImage = true; - return false; + return false; } }); From c3e266d9b9338344f646ce905108902851577765 Mon Sep 17 00:00:00 2001 From: Damani Brown Date: Tue, 12 May 2020 11:30:59 -0400 Subject: [PATCH 5/5] Code cleanup --- .../src/rules/MetadataIncludesOGImageSrc.ts | 2 +- packages/linter/tests/local.ts | 2 +- .../local/ImagesHaveAltText-1/source.html | 200 +----------------- .../local/ImagesHaveAltText-2/source.html | 200 +----------------- 4 files changed, 4 insertions(+), 400 deletions(-) diff --git a/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts b/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts index 12df29b26..104660414 100644 --- a/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts +++ b/packages/linter/src/rules/MetadataIncludesOGImageSrc.ts @@ -22,7 +22,7 @@ export class MetadataIncludesOGImageSrc extends Rule { } meta() { return { - url: "https://ogp.me/", + 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 31559965a..5c9cb337a 100644 --- a/packages/linter/tests/local.ts +++ b/packages/linter/tests/local.ts @@ -105,7 +105,7 @@ assertWarn( ); assertPass( - `${MetadataIncludesOGImageSrc.name} - is presentt`, + `${MetadataIncludesOGImageSrc.name} - is present`, runLocalTest( MetadataIncludesOGImageSrc, "local/MetadataIncludesOGImageSrc-1/source.html" diff --git a/packages/linter/tests/local/ImagesHaveAltText-1/source.html b/packages/linter/tests/local/ImagesHaveAltText-1/source.html index 03896941b..ab6efb0a9 100644 --- a/packages/linter/tests/local/ImagesHaveAltText-1/source.html +++ b/packages/linter/tests/local/ImagesHaveAltText-1/source.html @@ -75,204 +75,7 @@ Through the fence 2020 - - - - - - - - - - - - - - - - - - - + @@ -283,7 +86,6 @@

Through The Fence 2020

-
diff --git a/packages/linter/tests/local/ImagesHaveAltText-2/source.html b/packages/linter/tests/local/ImagesHaveAltText-2/source.html index 2552a578a..1b8f752db 100644 --- a/packages/linter/tests/local/ImagesHaveAltText-2/source.html +++ b/packages/linter/tests/local/ImagesHaveAltText-2/source.html @@ -75,204 +75,7 @@ Through the fence 2020 - - - - - - - - - - - - - - - - - - - + @@ -283,7 +86,6 @@

Through The Fence 2020

-