Add (and fix) SVG cleanliness tests

README.md

@@ -4,17 +4,27 @@
 
 <https://icons.cubing.net>
 
-## Development
+## Contributing
 
 Most development is simply adding/changing existing SVG files under the
-[`./src/svg` directory](https://github.com/cubing/icons/tree/main/src/svg). If
-you haven't dealt with SVG files before, check out
+[`./src/svg` directory](https://github.com/cubing/icons/tree/main/src/svg). 
+
+To add a new icon, please follow the following steps:
+
+1. Make or edit your icon! All icons should be vector SVG files. If
+you haven't dealt with SVGs before, check out
 [Inkscape](https://inkscape.org/).
+1. Make sure your SVG is cleaned up and minified. You can just upload your SVG file to [SVGOMG](https://jakearchibald.github.io/svgomg/) and then download the output file using the default settings.
+2. Name your icon in all lowercase, separating each word with an underscore. If specifying the dimensions of a cube in your icon, name, don't include any `x` symbols. (correct: `crazy_333.svg`, incorrect: `Crazy-3x3x3.SVG`)
+3. Remove any borders and fill colors from the your icon. To do this, look inside the SVG file and remove anything that looks like `fill="..."` or `stroke="..."`.
+4. Make a pull request here in GitHub adding your new icon file.
 
-If you want to actually build a font or CSS locally, you'll need some more tooling.
+Our maintainers can help you with any of the above steps if you need help.
 
 ### Build the project
 
+If you want to actually build a font or CSS locally, you'll need some more tooling.
+
 You'll need [`bun`](https://bun.sh) to install development dependencies and
 build the project. Either install `bun` with your preferred package manager, or
 use our [Nix-powered dev shell](#using-nix-for-development-optional).

script/test-svg.spec.ts

@@ -6,16 +6,34 @@ import { JSDOM } from "jsdom";
 // Collect into an array so we can iterate over it again multiple times.
 const svgFiles = await Array.fromAsync(new Glob("./src/svg/*/*.svg").scan());
 
-test("SVG files are all 500×500", async () => {
+/**
+ * Helper so TypeScript can infer `expect(...).not.toBeNull()`.
+ */
+function expectNotNull<T>(
+  value: T | null,
+  message: string,
+): asserts value is T {
+  expect(value, message).not.toBeNull();
+}
+
+test.concurrent("SVG files are all 500×500", async () => {
   let numSVGs = 0;
   for await (const svgFile of svgFiles) {
     numSVGs++;
     const svgElem = new JSDOM(
       await file(svgFile).text(),
     ).window.document.querySelector("svg");
-    expect(svgElem).not.toBeNull();
-    expect(svgElem?.getAttribute("width")).toEqual("500");
-    expect(svgElem?.getAttribute("height")).toEqual("500");
+    expectNotNull(svgElem, `${svgFile}: no svg`);
+    expect(svgElem?.getAttribute("width"), `${svgFile}: wrong width`).toEqual(
+      "500",
+    );
+    expect(svgElem?.getAttribute("height"), `${svgFile}: wrong height`).toEqual(
+      "500",
+    );
+    expect(
+      svgElem?.getAttribute("viewBox"),
+      `${svgFile}: wrong viewBox`,
+    ).toEqual("0 0 500 500");
   }
 
   /**
@@ -27,17 +45,19 @@ test("SVG files are all 500×500", async () => {
   expect(numSVGs).toBeGreaterThan(50);
 });
 
-test("SVG files follow naming conventions", async () => {
+test.concurrent("SVG files follow naming conventions", async () => {
   let numSVGs = 0;
   for await (const svgFile of svgFiles) {
     numSVGs++;
     const parentFolder = basename(dirname(svgFile));
     if (parentFolder === "penalty") {
-      expect(basename(svgFile)).toMatch(
+      expect(basename(svgFile), `${svgFile}: wrong basename`).toMatch(
         /^([A-Z]+\d+([a-z]+\d*)?|\d+[a-z]+(\d+[a-z]*)?)\.svg$/,
       );
     } else {
-      expect(basename(svgFile)).toMatch(/^[a-z0-9_]+\.svg$/);
+      expect(basename(svgFile), `${svgFile}: wrong basename`).toMatch(
+        /^[a-z0-9_]+\.svg$/,
+      );
     }
   }
 
@@ -49,3 +69,74 @@ test("SVG files follow naming conventions", async () => {
    */
   expect(numSVGs).toBeGreaterThan(50);
 });
+
+test.concurrent("SVGs have no hardcoded colors", async () => {
+  for await (const svgFile of svgFiles) {
+    function checkElement(el: SVGElement) {
+      expect(el.getAttribute("fill"), `${svgFile}: has fill color`).toBeNull();
+      expect(
+        el.getAttribute("stroke"),
+        `${svgFile}: has stroke color`,
+      ).toBeNull();
+      expect(
+        el.style.getPropertyValue("fill"),
+        `${svgFile}: has inline fill color`,
+      ).toBe("");
+      expect(
+        el.style.getPropertyValue("stroke"),
+        `${svgFile}: has inline stroke color`,
+      ).toBe("");
+
+      for (const child of el.children) {
+        checkElement(child as SVGElement);
+      }
+    }
+
+    const svgElem = new JSDOM(
+      await file(svgFile).text(),
+    ).window.document.querySelector("svg");
+
+    expectNotNull(svgElem, `${svgFile}: no svg`);
+    checkElement(svgElem);
+  }
+});
+
+test.concurrent("SVGs are well-formed with no extraneous attributes", async () => {
+  for await (const svgFile of svgFiles) {
+    const svgElem = new JSDOM(
+      await file(svgFile).text(),
+    ).window.document.querySelector("svg");
+
+    expectNotNull(svgElem, `${svgFile}: no svg`);
+    expect(svgElem.getAttribute("xmlns"), `${svgFile}: bad xmlns`).toBe(
+      "http://www.w3.org/2000/svg",
+    );
+    expect(
+      svgElem.getAttributeNames().sort(),
+      `${svgFile}: wrong attributes`,
+    ).toEqual(["width", "height", "viewBox", "xmlns"].sort());
+  }
+});
+
+const ALLOWED_ELEMENTS = ["svg", "g", "path", "circle", "defs"];
+
+test.concurrent("SVGs only have allowed elements", async () => {
+  for await (const svgFile of svgFiles) {
+    function checkElement(el: Element) {
+      expect(el.tagName, `${svgFile}: disallowed element`).toBeOneOf(
+        ALLOWED_ELEMENTS,
+      );
+
+      for (const child of el.children) {
+        checkElement(child as Element);
+      }
+    }
+
+    const svgElem = new JSDOM(
+      await file(svgFile).text(),
+    ).window.document.querySelector("svg");
+
+    expectNotNull(svgElem, `${svgFile}: no svg`);
+    checkElement(svgElem);
+  }
+});