-
Notifications
You must be signed in to change notification settings - Fork 80
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
Add check for images' alt text #2348
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
// This code is a Qiskit project. | ||
// | ||
// (C) Copyright IBM 2024. | ||
// | ||
// This code is licensed under the Apache License, Version 2.0. You may | ||
// obtain a copy of this license in the LICENSE file in the root directory | ||
// of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. | ||
// | ||
// Any modifications or derivative works of this code must retain this | ||
// copyright notice, and modified files need to carry a notice indicating | ||
// that they have been altered from the originals. | ||
|
||
import { globby } from "globby"; | ||
import { findImagesWithoutAltText } from "../lib/extractMarkdownImages.js"; | ||
import { readMarkdown } from "../lib/markdownReader.js"; | ||
|
||
async function main() { | ||
const files = await globby(["docs/**/*.{ipynb,mdx}", "!docs/api/**/*.mdx"]); | ||
const fileErrors: string[] = []; | ||
|
||
for (const file of files) { | ||
const markdown = await readMarkdown(file); | ||
const images = await findImagesWithoutAltText(markdown); | ||
const imageErrors = [...images].map( | ||
(image) => `The image '${image}' does not have an alt text defined.`, | ||
); | ||
|
||
if (imageErrors.length) { | ||
fileErrors.push( | ||
`Error in file '${file}':\n\t- ${imageErrors.join("\n\t- ")}`, | ||
); | ||
} | ||
} | ||
|
||
if (fileErrors.length) { | ||
fileErrors.forEach((error) => console.log(error)); | ||
console.error("\nSome images are missing alt text 💔\n"); | ||
process.exit(1); | ||
} | ||
console.log("\nAll images have alt text ✅\n"); | ||
} | ||
|
||
main().then(() => process.exit()); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
// This code is a Qiskit project. | ||
// | ||
// (C) Copyright IBM 2023. | ||
// | ||
// This code is licensed under the Apache License, Version 2.0. You may | ||
// obtain a copy of this license in the LICENSE file in the root directory | ||
// of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. | ||
// | ||
// Any modifications or derivative works of this code must retain this | ||
// copyright notice, and modified files need to carry a notice indicating | ||
// that they have been altered from the originals. | ||
|
||
import { expect, test } from "@playwright/test"; | ||
|
||
import { findImagesWithoutAltText } from "./extractMarkdownImages.js"; | ||
|
||
test("Test the extraction of the images", async () => { | ||
const markdown = ` | ||
# A header | ||
|
||
![Our first image with alt text](/images/img1.png) | ||
|
||
![](/images/invalid_img1.png) | ||
|
||
![Here's another valid image](/images/img2.png) | ||
|
||
![](/images/invalid_img2.png) | ||
|
||
![](/images/invalid_img2.png) | ||
|
||
![And now, our last link](https://ibm.com) | ||
`; | ||
const images = await findImagesWithoutAltText(markdown); | ||
const correct_images = new Set([ | ||
"/images/invalid_img1.png", | ||
"/images/invalid_img2.png", | ||
]); | ||
|
||
expect(images).toEqual(correct_images); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
// This code is a Qiskit project. | ||
// | ||
// (C) Copyright IBM 2024. | ||
// | ||
// This code is licensed under the Apache License, Version 2.0. You may | ||
// obtain a copy of this license in the LICENSE file in the root directory | ||
// of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. | ||
// | ||
// Any modifications or derivative works of this code must retain this | ||
// copyright notice, and modified files need to carry a notice indicating | ||
// that they have been altered from the originals. | ||
|
||
import { unified } from "unified"; | ||
import { Root } from "remark-mdx"; | ||
import { visit } from "unist-util-visit"; | ||
import remarkParse from "remark-parse"; | ||
import remarkGfm from "remark-gfm"; | ||
import remarkStringify from "remark-stringify"; | ||
|
||
export async function findImagesWithoutAltText( | ||
markdown: string, | ||
): Promise<Set<string>> { | ||
const images = new Set<string>(); | ||
|
||
await unified() | ||
.use(remarkParse) | ||
.use(remarkGfm) | ||
.use(() => (tree: Root) => { | ||
visit(tree, "image", (node) => { | ||
if (!node.alt) { | ||
images.add(node.url); | ||
} | ||
}); | ||
Comment on lines
+29
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI: In my opinion, the visitor is already very simple and readable, but I discovered that we can add more conditions to the visit(
tree,
(node) => node.type === "image" && !node.alt,
(node) => images.add(node.url),
); I don't think is useful right now, but we could use it in the future if the logic becomes more complex. |
||
}) | ||
.use(remarkStringify) | ||
.process(markdown); | ||
Comment on lines
+25
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Notice that the plugin will not extract images from HTML tags. I have searched in the repo and we don't have any instance of it. HTML tags can be used for the writers if they want to resize the images in markdown: <img src="example.jpg" alt="Example" width="200"/> We could implement this by adding another visitor that uses Cheerio as follows: visit(tree, "html", (node) => {
const $ = load(node.value);
const imgElement = $("img");
if (imgElement.length) {
images.add(imgElement.attr("src"));
}
}); I think it would be good to add it to make it more robust, but what do you think? |
||
|
||
return images; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This image was missing the alt text. Looking at the notebook (https://docs.quantum.ibm.com/guides/custom-transpiler-pass) I think the alt text of the previous image (the circuit inside the "Background: DAG representation" collapsible) refereed to this one, so I moved it here and added an alt text for the circuit