fix: Color parsing supports hex strings without hash [135] (#138)

Author: lanesawyer

packages/core/src/colors.ts

@@ -1,20 +1,39 @@
 import type { vec3, vec4 } from '@alleninstitute/vis-geometry';
 import { logger } from './logger';
 
-const RGB_COLOR_REGEX = /^#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/;
-const RGBA_COLOR_REGEX = /^#([0-9a-fA-F]{4}|[0-9a-fA-F]{8})$/;
+// Tests for optional #, then 3 or 6 hex digits
+const RGB_COLOR_REGEX = /^#?([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/;
+// Tests for optional #, then 4 or 8 hex digits
+const RGBA_COLOR_REGEX = /^#?([0-9a-fA-F]{4}|[0-9a-fA-F]{8})$/;
 
+/**
+ * Converts a color hash string to a vec3 RGB color vector.
+ *
+ * @param colorHashStr A string representing a color in hex format, e.g., '#f00' or 'ff0000'.
+ * @param normalized A boolean indicating whether to normalize the color values to the range [0, 1]. Defaults to true.
+ * @returns A vec3 array representing the RGB color vector. If the input is invalid, returns [0, 0, 0].
+ */
 export function makeRGBColorVector(colorHashStr: string, normalized = true): vec3 {
     if (!colorHashStr || !RGB_COLOR_REGEX.test(colorHashStr)) {
         logger.warn('invalid color hash string; returning black color vector (0, 0, 0)');
         return [0, 0, 0];
     }
+    const hasHash = colorHashStr.charAt(0) === '#';
+    const sanitizedColorHashStr = hasHash ? colorHashStr : `#${colorHashStr}`;
+
     const redCode =
-        colorHashStr.length === 4 ? colorHashStr.charAt(1) + colorHashStr.charAt(1) : colorHashStr.slice(1, 3);
+        sanitizedColorHashStr.length === 4
+            ? sanitizedColorHashStr.charAt(1) + sanitizedColorHashStr.charAt(1)
+            : sanitizedColorHashStr.slice(1, 3);
     const greenCode =
-        colorHashStr.length === 4 ? colorHashStr.charAt(2) + colorHashStr.charAt(2) : colorHashStr.slice(3, 5);
+        sanitizedColorHashStr.length === 4
+            ? sanitizedColorHashStr.charAt(2) + sanitizedColorHashStr.charAt(2)
+            : sanitizedColorHashStr.slice(3, 5);
     const blueCode =
-        colorHashStr.length === 4 ? colorHashStr.charAt(3) + colorHashStr.charAt(3) : colorHashStr.slice(5, 7);
+        sanitizedColorHashStr.length === 4
+            ? sanitizedColorHashStr.charAt(3) + sanitizedColorHashStr.charAt(3)
+            : sanitizedColorHashStr.slice(5, 7);
+
     const divisor = normalized ? 255 : 1;
     return [
         Number.parseInt(redCode, 16) / divisor,
@@ -23,20 +42,39 @@ export function makeRGBColorVector(colorHashStr: string, normalized = true): vec
     ];
 }
 
+/**
+ * Converts a color hash string to a vec4 RGBA color vector.
+ *
+ * @param colorHashStr A string representing a color in hex format, e.g., '#f00f' or 'ff0000ff'.
+ * @param normalized A boolean indicating whether to normalize the color values to the range [0, 1]. Defaults to true.
+ * @returns A vec3 array representing the RGB color vector. If the input is invalid, returns [0, 0, 0, 0].
+ */
 export function makeRGBAColorVector(colorHashStr: string, normalized = true): vec4 {
     if (!colorHashStr) {
         logger.warn('invalid color hash string; returning transparent black color vector (0, 0, 0, 0)');
         return [0, 0, 0, 0];
     }
+
     if (RGBA_COLOR_REGEX.test(colorHashStr)) {
+        const hashHash = colorHashStr.charAt(0) === '#';
+        const sanitizedColorHashStr = hashHash ? colorHashStr : `#${colorHashStr}`;
+
         const redCode =
-            colorHashStr.length === 5 ? colorHashStr.charAt(1) + colorHashStr.charAt(1) : colorHashStr.slice(1, 3);
+            sanitizedColorHashStr.length === 5
+                ? sanitizedColorHashStr.charAt(1) + sanitizedColorHashStr.charAt(1)
+                : sanitizedColorHashStr.slice(1, 3);
         const greenCode =
-            colorHashStr.length === 5 ? colorHashStr.charAt(2) + colorHashStr.charAt(2) : colorHashStr.slice(3, 5);
+            sanitizedColorHashStr.length === 5
+                ? sanitizedColorHashStr.charAt(2) + sanitizedColorHashStr.charAt(2)
+                : sanitizedColorHashStr.slice(3, 5);
         const blueCode =
-            colorHashStr.length === 5 ? colorHashStr.charAt(3) + colorHashStr.charAt(3) : colorHashStr.slice(5, 7);
+            sanitizedColorHashStr.length === 5
+                ? sanitizedColorHashStr.charAt(3) + sanitizedColorHashStr.charAt(3)
+                : sanitizedColorHashStr.slice(5, 7);
         const alphaCode =
-            colorHashStr.length === 5 ? colorHashStr.charAt(4) + colorHashStr.charAt(4) : colorHashStr.slice(7, 9);
+            sanitizedColorHashStr.length === 5
+                ? sanitizedColorHashStr.charAt(4) + sanitizedColorHashStr.charAt(4)
+                : sanitizedColorHashStr.slice(7, 9);
         const divisor = normalized ? 255 : 1;
         return [
             Number.parseInt(redCode, 16) / divisor,
@@ -46,8 +84,8 @@ export function makeRGBAColorVector(colorHashStr: string, normalized = true): ve
         ];
     }
     if (RGB_COLOR_REGEX.test(colorHashStr)) {
-        const rgb = makeRGBColorVector(colorHashStr);
-        return [rgb[0], rgb[1], rgb[2], normalized ? 1.0 : 255.0];
+        const rgb = makeRGBColorVector(colorHashStr, normalized);
+        return [...rgb, normalized ? 1.0 : 255.0];
     }
     logger.warn('invalid color hash string; returning transparent black color vector (0, 0, 0, 0)');
     return [0, 0, 0, 0];

packages/core/src/test/colors.test.ts

@@ -0,0 +1,86 @@
+import { describe, it, expect, vi } from 'vitest';
+import { makeRGBColorVector, makeRGBAColorVector } from '../colors';
+import { logger } from '../logger';
+
+// Mock the logger to verify warning logs are emitted
+vi.mock('../logger', () => ({
+    logger: {
+        warn: vi.fn(),
+    },
+}));
+
+describe('makeRGBColorVector', () => {
+    it('should return a black for invalid input and log warning', () => {
+        const result = makeRGBColorVector('this is not a color');
+        expect(result).toEqual([0, 0, 0]);
+        expect(logger.warn).toHaveBeenCalled();
+    });
+
+    it('should handle 3-character RGB without a hash', () => {
+        const result = makeRGBColorVector('f00');
+        expect(result).toEqual([1, 0, 0]);
+    });
+
+    it('should handle 3-character RGB with a hash', () => {
+        const result = makeRGBColorVector('#f00');
+        expect(result).toEqual([1, 0, 0]);
+    });
+
+    it('should handle 6-character RGB without a hash', () => {
+        const result = makeRGBColorVector('ff0000');
+        expect(result).toEqual([1, 0, 0]);
+    });
+
+    it('should handle 6-character RGB with a hash', () => {
+        const result = makeRGBColorVector('#ff0000');
+        expect(result).toEqual([1, 0, 0]);
+    });
+
+    it('should return non-normalized values when normalize is false', () => {
+        const result = makeRGBColorVector('#ff0000', false);
+        expect(result).toEqual([255, 0, 0]);
+    });
+});
+
+describe('makeRGBAColorVector', () => {
+    it('should return a transparent black for invalid input and log warning', () => {
+        const result = makeRGBAColorVector('this is not a color');
+        expect(result).toEqual([0, 0, 0, 0]);
+        expect(logger.warn).toHaveBeenCalled();
+    });
+
+    it('should handle 4-character RGBA without a hash', () => {
+        const result = makeRGBAColorVector('f00f');
+        expect(result).toEqual([1, 0, 0, 1]);
+    });
+
+    it('should handle 4-character RGBA with a hash', () => {
+        const result = makeRGBAColorVector('#f00f');
+        expect(result).toEqual([1, 0, 0, 1]);
+    });
+
+    it('should handle 8-character RGBA without a hash', () => {
+        const result = makeRGBAColorVector('ff0000ff');
+        expect(result).toEqual([1, 0, 0, 1]);
+    });
+
+    it('should handle 8-character RGBA with a hash', () => {
+        const result = makeRGBAColorVector('#ff0000ff');
+        expect(result).toEqual([1, 0, 0, 1]);
+    });
+
+    it('should handle RGB input and add alpha channel', () => {
+        const result = makeRGBAColorVector('#ff0000');
+        expect(result).toEqual([1, 0, 0, 1]);
+    });
+
+    it('should return non-normalized values when normalize is false', () => {
+        const result = makeRGBAColorVector('#ff0000ff', false);
+        expect(result).toEqual([255, 0, 0, 255]);
+    });
+
+    it('should handle RGB input and add non-normalized alpha channel when normalize is false', () => {
+        const result = makeRGBAColorVector('#ff0000', false);
+        expect(result).toEqual([255, 0, 0, 255]);
+    });
+});