rework based on PR suggestions

Khan · Oct 17, 2024 · 79844ca · 79844ca
1 parent 0618a28
commit 79844ca
Show file tree

Hide file tree

Showing 8 changed files with 78 additions and 61 deletions.
diff --git a/...us-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx b/...us-editor/src/widgets/interactive-graph-editor/locked-figures/locked-ellipse-settings.tsx
@@ -88,7 +88,7 @@ const LockedEllipseSettings = (props: Props) => {
             strokeStyle,
             fillStyle,
         );
-        str += `${ellipseAppearance}`;
+        str += ellipseAppearance;
         return str;
     }
 

diff --git a/...s-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.tsx b/...s-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.tsx
@@ -102,7 +102,7 @@ const LockedFunctionSettings = (props: Props) => {
             lineColor,
             strokeStyle,
         );
-        str += `${functionAppearance}`;
+        str += functionAppearance;
 
         return str;
     }

diff --git a/...rseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx b/...rseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-line-settings.tsx
@@ -84,7 +84,7 @@ const LockedLineSettings = (props: Props) => {
             lineColor,
             lineStyle,
         );
-        str += `${lineAppearance}`;
+        str += lineAppearance;
 
         return str;
     }

diff --git a/...seus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.tsx b/...seus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-point-settings.tsx
@@ -122,7 +122,7 @@ const LockedPointSettings = (props: Props) => {
 
         const pointAppearance =
             generateLockedFigureAppearanceDescription(pointColor);
-        str += `${pointAppearance}`;
+        str += pointAppearance;
 
         return str;
     }

diff --git a/...us-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx b/...us-editor/src/widgets/interactive-graph-editor/locked-figures/locked-polygon-settings.tsx
@@ -80,7 +80,7 @@ const LockedPolygonSettings = (props: Props) => {
             strokeStyle,
             fillStyle,
         );
-        str += `${polygonAppearance}`;
+        str += polygonAppearance;
         return str;
     }
 

diff --git a/...eus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx b/...eus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-vector-settings.tsx
@@ -74,7 +74,7 @@ const LockedVectorSettings = (props: Props) => {
 
         const vectorAppearance =
             generateLockedFigureAppearanceDescription(lineColor);
-        str += `${vectorAppearance}`;
+        str += vectorAppearance;
 
         return str;
     }

diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.test.ts b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.test.ts
@@ -112,61 +112,54 @@ describe("getDefaultFigureForType", () => {
 
 describe("generateLockedFigureAppearanceDescription", () => {
     // one argument
-    test.each([
-        ["grayH"],
-        ["red"],
-        ["blue"],
-        ["green"],
-        ["purple"],
-        ["orange"],
-        ["pink"],
-    ])(
-        `should return a string with color of %s and a solid stroke style`,
+    test(`should return a string with a gray color and a solid stroke style`, () => {
+        const description = generateLockedFigureAppearanceDescription("grayH");
+        expect(description).toBe(`. Appearance solid gray.`);
+    });
+
+    test.each([["red"], ["blue"], ["green"], ["purple"], ["orange"]])(
+        `should return a string with a %s color and a solid stroke style`,
         (color: LockedFigureColor) => {
             const description =
                 generateLockedFigureAppearanceDescription(color);
-            const convertedColor = color === "grayH" ? "gray" : color;
-            expect(description).toBe(`. Appearance solid ${convertedColor}.`);
+
+            expect(description).toBe(`. Appearance solid ${color}.`);
         },
     );
 
     // two arguments
     test.each([
-        ["grayH", "solid"],
-        ["red", "solid"],
         ["blue", "solid"],
-        ["green", "solid"],
-        ["purple", "solid"],
-        ["orange", "solid"],
-        ["pink", "solid"],
-        ["grayH", "dashed"],
-        ["red", "dashed"],
-        ["blue", "dashed"],
         ["green", "dashed"],
-        ["purple", "dashed"],
-        ["orange", "dashed"],
-        ["pink", "dashed"],
     ])(
         `should return a string with color of %s and a stroke style of %s`,
         (color: LockedFigureColor, strokeStyle: LockedLineStyle) => {
             const description = generateLockedFigureAppearanceDescription(
                 color,
                 strokeStyle,
             );
-            const convertedColor = color === "grayH" ? "gray" : color;
-            expect(description).toBe(
-                `. Appearance ${strokeStyle} ${convertedColor}.`,
-            );
+            expect(description).toBe(`. Appearance ${strokeStyle} ${color}.`);
         },
     );
 
     // three arguments
+    // no fill
+    test(`should return a string with a gray color, solid stroke, and no fill`, () => {
+        const description = generateLockedFigureAppearanceDescription(
+            "grayH",
+            undefined,
+            "none",
+        );
+        expect(description).toBe(
+            `. Appearance solid gray border, with no fill.`,
+        );
+    });
+
     test.each([
-        ["grayH", "solid", "none"],
+        ["blue", "solid", "none"],
         ["red", "dashed", "none"],
-        ["blue", undefined, "none"],
     ])(
-        `should return a string with a color, stroke, and fill when given color input of %s, stroke input of %s, and fill input of none`,
+        `should return a string with a %s color, %s stroke and no fill`,
         (
             color: LockedFigureColor,
             strokeStyle: LockedLineStyle,
@@ -177,22 +170,30 @@ describe("generateLockedFigureAppearanceDescription", () => {
                 strokeStyle,
                 fill,
             );
-            const convertedColor = color === "grayH" ? "gray" : color;
-            const convertedFill = fill === "none" ? "no" : `${fill}`;
-            const convertedStroke = strokeStyle ? strokeStyle : "solid";
 
             expect(description).toBe(
-                `. Appearance ${convertedStroke} ${convertedColor} border, with ${convertedFill} fill.`,
+                `. Appearance ${strokeStyle} ${color} border, with no fill.`,
             );
         },
     );
 
+    // white fill
+    test("should return a string with a gray color, solid stroke, and a white fill", () => {
+        const description = generateLockedFigureAppearanceDescription(
+            "grayH",
+            undefined,
+            "white",
+        );
+        expect(description).toBe(
+            `. Appearance solid gray border, with a white fill.`,
+        );
+    });
+
     test.each([
-        ["grayH", "solid", "white"],
+        ["pink", "solid", "white"],
         ["red", "dashed", "white"],
-        ["blue", undefined, "white"],
     ])(
-        `should return a string with a color, stroke, and fill when given color input of %s, stroke input of %s, and fill input of white`,
+        `should return a string with a %s color, %s stroke and a white fill`,
         (
             color: LockedFigureColor,
             strokeStyle: LockedLineStyle,
@@ -203,25 +204,43 @@ describe("generateLockedFigureAppearanceDescription", () => {
                 strokeStyle,
                 fill,
             );
-            const convertedColor = color === "grayH" ? "gray" : color;
-            const convertedFill = fill === "none" ? "no" : `${fill}`;
-            const convertedStroke = strokeStyle ? strokeStyle : "solid";
 
             expect(description).toBe(
-                `. Appearance ${convertedStroke} ${convertedColor} border, with a ${convertedFill} fill.`,
+                `. Appearance ${strokeStyle} ${color} border, with a white fill.`,
             );
         },
     );
 
+    // solid and translucent fills
+    test("should return a string with a gray color, solid stroke, and a solid fill", () => {
+        const description = generateLockedFigureAppearanceDescription(
+            "grayH",
+            undefined,
+            "solid",
+        );
+        expect(description).toBe(
+            `. Appearance solid gray border, with a solid gray fill.`,
+        );
+    });
+
+    test("should return a string with a gray color, solid stroke, and a translucent fill", () => {
+        const description = generateLockedFigureAppearanceDescription(
+            "pink",
+            undefined,
+            "translucent",
+        );
+        expect(description).toBe(
+            `. Appearance solid pink border, with a translucent pink fill.`,
+        );
+    });
+
     test.each([
-        ["grayH", "solid", "solid"],
+        ["pink", "solid", "solid"],
         ["red", "dashed", "solid"],
         ["green", "dashed", "translucent"],
         ["purple", "solid", "translucent"],
-        ["grayH", undefined, "solid"],
-        ["red", undefined, "translucent"],
     ])(
-        `should return a string with a color, stroke, and fill when given color input of %s, stroke input of %s, and fill input of %s`,
+        `should return a string with a %s color, %s stroke, and a %s fill`,
         (
             color: LockedFigureColor,
             strokeStyle: LockedLineStyle,
@@ -232,12 +251,9 @@ describe("generateLockedFigureAppearanceDescription", () => {
                 strokeStyle,
                 fill,
             );
-            const convertedColor = color === "grayH" ? "gray" : color;
-            const convertedFill = fill === "none" ? "no" : `${fill}`;
-            const convertedStroke = strokeStyle ? strokeStyle : "solid";
 
             expect(description).toBe(
-                `. Appearance ${convertedStroke} ${convertedColor} border, with a ${convertedFill} ${convertedColor} fill.`,
+                `. Appearance ${strokeStyle} ${color} border, with a ${fill} ${color} fill.`,
             );
         },
     );

diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.ts b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.ts
@@ -109,17 +109,18 @@ export function generateLockedFigureAppearanceDescription(
     fill?: LockedFigureFillType,
 ) {
     const convertedColor = color === "grayH" ? "gray" : color;
-    const convertedFill = fill === "none" ? "no" : `${fill}`;
 
     switch (fill) {
         case "none":
-            return `. Appearance ${strokeStyle} ${convertedColor} border, with ${convertedFill} fill.`;
+            return `. Appearance ${strokeStyle} ${convertedColor} border, with no fill.`;
         case "white":
-            return `. Appearance ${strokeStyle} ${convertedColor} border, with a ${convertedFill} fill.`;
+            return `. Appearance ${strokeStyle} ${convertedColor} border, with a white fill.`;
         case "solid":
         case "translucent":
-            return `. Appearance ${strokeStyle} ${convertedColor} border, with a ${convertedFill} ${convertedColor} fill.`;
-        default:
+            return `. Appearance ${strokeStyle} ${convertedColor} border, with a ${fill} ${convertedColor} fill.`;
+        case undefined:
             return `. Appearance ${strokeStyle} ${convertedColor}.`;
+        default:
+            throw new UnreachableCaseError(fill);
     }
 }