wip by benhalverson · Pull Request #39 · benhalverson/store

benhalverson · 2026-03-07T10:04:17Z

No description provided.

Copilot

Pull request overview

This PR introduces a versioned ColorPicker implementation (v1 vs v2) behind an environment-driven feature flag, and updates the shared color data model so the UI can consume either API shape consistently.

Changes:

Add COLOR_PICKER_VERSION config and a ColorPickerWrapper to select v1/v2 at runtime.
Introduce ColorPickerV2 for the /v2/colors endpoint and map the v1 /colors response into the new normalized ColorsResponse shape.
Update color types/context to use hexValue and adjust impacted tests/mocks.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
src/pages/Product.tsx	Switches product page to render the new `ColorPickerWrapper`.
src/pages/Product.test.tsx	Extends config mock with `COLOR_PICKER_VERSION` for the new wrapper behavior.
src/interfaces/colors.ts	Replaces the old v1 `/colors` interface with a normalized/v2-oriented shape.
src/context/ColorContext.tsx	Updates initial selected color logic to use `hexValue`.
src/config.ts	Adds env parsing/export for `VITE_COLOR_PICKER_VERSION` with default `"v1"`.
src/components/ColorPickerWrapper.tsx	New wrapper that selects v1 vs v2 picker by feature flag.
src/components/ColorPickerV2.tsx	New v2 picker that fetches from `/v2/colors` and dispatches options to context.
src/components/ColorPicker.tsx	Updates v1 picker to map legacy response into the new `ColorsResponse` shape and use `hexValue`.
src/components/ColorPicker.test.tsx	Updates mocks to reflect the v1 `/colors` response shape.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-07T10:08:59Z

src/pages/Product.test.tsx

 vi.mock("../config", () => ({
  BASE_URL: "http://test.local",
  DOMAIN: "http://test.local",
+  COLOR_PICKER_VERSION: "v1",
 }));


ProductPage now renders ColorPickerWrapper, but this test file only mocks ../components/ColorPicker. Depending on Vitest module ID resolution, the wrapper may still render the real picker and trigger an extra /colors fetch (conflicting with the product fetch mock). Consider mocking ../components/ColorPickerWrapper directly (or making the fetch mock branch on URL) to keep the page tests isolated.

Copilot · 2026-03-07T10:09:00Z

src/config.ts

 const parsed = zEnv.parse(import.meta.env);

 export const BASE_URL = parsed.VITE_BASE_URL;
 export const DOMAIN = parsed.VITE_DOMAIN;


A new env var (VITE_COLOR_PICKER_VERSION) is introduced here, but it isn't present in .env.sample (and may not be documented elsewhere). Consider updating the sample env/README so local/dev deployments discover the flag and valid values (v1/v2).

Suggested change

export const DOMAIN = parsed.VITE_DOMAIN;

export const DOMAIN = parsed.VITE_DOMAIN;

/**

* Color picker implementation version selector.

*

* Backed by the `VITE_COLOR_PICKER_VERSION` env var.

* Valid values:

* - "v1" (default)

* - "v2"

*

* When `VITE_COLOR_PICKER_VERSION` is not set, "v1" is used by default.

*/

Copilot · 2026-03-07T10:09:00Z

src/components/ColorPicker.tsx

+          public: true,
+          available: true,
+          profile: c.colorTag,
+          color: c.filament,


The v1→ColorsResponse mapping sets color (used for the option's accessible label) to c.filament, which will make every radio option label identical (e.g. all "PLA") and loses the actual color name (likely colorTag). Map the display/label field(s) from colorTag instead so options remain distinguishable and accessible.

Suggested change

color: c.filament,

color: c.colorTag,

Copilot · 2026-03-07T10:09:00Z

src/components/ColorPicker.tsx

        const url = new URL(`${BASE_URL}/colors`);
        if (filamentType) url.searchParams.set("filamentType", filamentType);

        const response = await fetch(url.toString());
-        const colors = (await response.json()) as ColorsResponse[];
-        dispatch({ type: "SET_COLOR_OPTIONS", payload: colors });
+
+        // v1 /colors response shape
+        type V1ColorResponse = {
+          filament: string;
+          hexColor: string;
+          colorTag: string;
+        };
+
+        const rawColors = (await response.json()) as V1ColorResponse[];
+


This fetch path parses JSON and immediately treats it as an array; if the server returns a non-2xx response (or an error object), rawColors.map will throw at runtime. Consider checking response.ok (and/or validating Array.isArray on the decoded payload) before mapping/dispatching.

Copilot · 2026-03-07T10:09:00Z

src/components/ColorPicker.tsx

+        console.log("Fetched colors (v1 mapped):", mappedColors);
+        dispatch({ type: "SET_COLOR_OPTIONS", payload: mappedColors });
      } catch (error) {
        console.error("Failed to fetch colors:", error);


Avoid leaving console.log in the shipping UI component; it will spam browser logs and can leak potentially sensitive data. Prefer removing it or gating behind a debug flag.

Suggested change

console.log("Fetched colors (v1 mapped):", mappedColors);

dispatch({ type: "SET_COLOR_OPTIONS", payload: mappedColors });

} catch (error) {

console.error("Failed to fetch colors:", error);

dispatch({ type: "SET_COLOR_OPTIONS", payload: mappedColors });

} catch (error) {

console.error("Failed to fetch colors:", error);

console.error("Failed to fetch colors:", error);

Copilot · 2026-03-07T10:09:01Z

src/components/ColorPickerV2.tsx

+        const json = (await response.json()) as V2ColorsEnvelope;
+        const colors = json.data ?? [];
+


fetch response handling doesn’t check response.ok and assumes the body matches V2ColorsEnvelope. If the endpoint returns non-2xx or an error shape, this will either throw or silently set colorOptions incorrectly. Add an ok/status check and validate json.success/json.data before dispatching.

Suggested change

const json = (await response.json()) as V2ColorsEnvelope;

const colors = json.data ?? [];

if (!response.ok) {

console.error(

`Failed to fetch v2 colors: ${response.status} ${response.statusText}`

);

dispatch({ type: "SET_COLOR_OPTIONS", payload: [] });

return;

}

const json = (await response.json()) as Partial<V2ColorsEnvelope>;

if (!json || json.success !== true || !Array.isArray(json.data)) {

console.error(

"Unexpected v2 colors response shape or unsuccessful response:",

json

);

dispatch({ type: "SET_COLOR_OPTIONS", payload: [] });

return;

}

const colors = json.data;

Copilot · 2026-03-07T10:09:01Z

src/components/ColorPickerV2.tsx

+        console.log("Fetched colors (v2):", colors);
+        dispatch({ type: "SET_COLOR_OPTIONS", payload: colors });
+      } catch (error) {
+        console.error("Failed to fetch v2 colors:", error);


Avoid leaving console.log statements in the component; it will clutter production logs for every product page visit. Remove it or guard it behind a debug flag.

Suggested change

console.log("Fetched colors (v2):", colors);

dispatch({ type: "SET_COLOR_OPTIONS", payload: colors });

} catch (error) {

console.error("Failed to fetch v2 colors:", error);

dispatch({ type: "SET_COLOR_OPTIONS", payload: colors });

} catch (error) {

console.error("Failed to fetch v2 colors:", error);

console.error("Failed to fetch v2 colors:", error);

Copilot · 2026-03-07T10:09:01Z

src/components/ColorPickerWrapper.tsx

+const ColorPickerWrapper: React.FC<Props> = ({ filamentType }) => {
+  if (COLOR_PICKER_VERSION === "v2") {
+    return <ColorPickerV2 filamentType={filamentType} />;
+  }
+
+  // Default to v1 implementation
+  return <ColorPicker filamentType={filamentType} />;
+};


New ColorPicker version selection logic is introduced, but there’s no test coverage ensuring the wrapper renders v1 vs v2 based on COLOR_PICKER_VERSION. Adding a small unit test for ColorPickerWrapper would help prevent accidental regressions when toggling versions.

Copilot · 2026-03-07T10:09:01Z

src/components/ColorPickerWrapper.tsx

+import { COLOR_PICKER_VERSION } from "../config";
+import ColorPicker from "./ColorPicker";
+import ColorPickerV2 from "./ColorPickerV2";
+
+interface Props {
+  filamentType: string;
+}
+
+const ColorPickerWrapper: React.FC<Props> = ({ filamentType }) => {
+  if (COLOR_PICKER_VERSION === "v2") {
+    return <ColorPickerV2 filamentType={filamentType} />;
+  }
+
+  // Default to v1 implementation
+  return <ColorPicker filamentType={filamentType} />;


ColorPickerWrapper imports both v1 and v2 implementations eagerly, so the v2 code ships even when COLOR_PICKER_VERSION is "v1". If bundle size matters, consider lazy-loading the inactive implementation (e.g., via React.lazy or dynamic import()), especially since the wrapper is on a core page.

Suggested change

import { COLOR_PICKER_VERSION } from "../config";

import ColorPicker from "./ColorPicker";

import ColorPickerV2 from "./ColorPickerV2";

interface Props {

filamentType: string;

}

const ColorPickerWrapper: React.FC<Props> = ({ filamentType }) => {

if (COLOR_PICKER_VERSION === "v2") {

return <ColorPickerV2 filamentType={filamentType} />;

}

// Default to v1 implementation

return <ColorPicker filamentType={filamentType} />;

import { lazy, Suspense } from "react";

import { COLOR_PICKER_VERSION } from "../config";

const ColorPicker = lazy(() => import("./ColorPicker"));

const ColorPickerV2 = lazy(() => import("./ColorPickerV2"));

interface Props {

filamentType: string;

}

const ColorPickerWrapper: React.FC<Props> = ({ filamentType }) => {

if (COLOR_PICKER_VERSION === "v2") {

return (

<Suspense fallback={null}>

<ColorPickerV2 filamentType={filamentType} />

</Suspense>

);

}

// Default to v1 implementation

return (

<Suspense fallback={null}>

<ColorPicker filamentType={filamentType} />;

</Suspense>

);

Copilot · 2026-03-07T10:09:01Z

src/interfaces/colors.ts

 /**
 * The response for the /colors endpoint
 */
 export interface ColorsResponse {
-  filament: string;
-  hexColor: string;
-  colorTag: string;
+  name: string;
+  provider: string;


The interface doc comment says this is the response shape for /colors, but the fields now correspond to the new normalized/v2 shape (and the v1 /colors response is different and is being mapped in ColorPicker). Update the comment to avoid misleading future callers.

wip

a4826ca

Copilot AI review requested due to automatic review settings March 7, 2026 10:04

Copilot started reviewing on behalf of benhalverson March 7, 2026 10:04 View session

Copilot AI reviewed Mar 7, 2026

View reviewed changes

benhalverson merged commit 9be311a into main Mar 7, 2026
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wip#39

wip#39
benhalverson merged 1 commit intomainfrom
update-apis

benhalverson commented Mar 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-export const DOMAIN = parsed.VITE_DOMAIN;
+export const DOMAIN = parsed.VITE_DOMAIN;
+/**
+ * Color picker implementation version selector.
+ *
+ * Backed by the `VITE_COLOR_PICKER_VERSION` env var.
+ * Valid values:
+ *   - "v1" (default)
+ *   - "v2"
+ *
+ * When `VITE_COLOR_PICKER_VERSION` is not set, "v1" is used by default.
+ */

		const json = (await response.json()) as V2ColorsEnvelope;
		const colors = json.data ?? [];

-        const json = (await response.json()) as V2ColorsEnvelope;
-        const colors = json.data ?? [];
+        if (!response.ok) {
+          console.error(
+            `Failed to fetch v2 colors: ${response.status} ${response.statusText}`
+          );
+          dispatch({ type: "SET_COLOR_OPTIONS", payload: [] });
+          return;
+        }
+        const json = (await response.json()) as Partial<V2ColorsEnvelope>;
+        if (!json || json.success !== true || !Array.isArray(json.data)) {
+          console.error(
+            "Unexpected v2 colors response shape or unsuccessful response:",
+            json
+          );
+          dispatch({ type: "SET_COLOR_OPTIONS", payload: [] });
+          return;
+        }
+        const colors = json.data;

Conversation

benhalverson commented Mar 7, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants