From 33740af5eed11fc773ee6e24fa5603ba7e4388c7 Mon Sep 17 00:00:00 2001
From: Matchu <matchu1993@gmail.com>
Date: Mon, 1 Nov 2021 16:19:17 -0700
Subject: [PATCH] Stop passing null value in SpeciesColorPicker

This is a minor change to clear a console warning, and make intended behavior clearer! You're not supposed to pass `null` as a select value, because it's ambiguous about whether you're looking for the first option or to make this an "uncontrolled component".

Here, I now provide a fallback value, which is an explicit string for the placeholder option. I made the string very explicit, to aid in debugging if it somehow leaks out from where it's supposed to be! (But I also added gating in the `onChange` event, just to be extra sure.)
---
 src/app/components/SpeciesColorPicker.js | 26 ++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/src/app/components/SpeciesColorPicker.js b/src/app/components/SpeciesColorPicker.js
index 5746a03..0dd915f 100644
--- a/src/app/components/SpeciesColorPicker.js
+++ b/src/app/components/SpeciesColorPicker.js
@@ -89,6 +89,13 @@ function SpeciesColorPicker({
       newColorId,
     });
 
+    // Ignore switching to the placeholder option. It shouldn't generally be
+    // doable once real options exist, and it doesn't represent a valid or
+    // meaningful transition in the case where it could happen.
+    if (newColorId === "SpeciesColorPicker-color-loading-placeholder") {
+      return;
+    }
+
     const species = allSpecies.find((s) => s.id === speciesId);
     const newColor = allColors.find((c) => c.id === newColorId);
     const validPoses = getValidPoses(valids, speciesId, newColorId);
@@ -119,6 +126,13 @@ function SpeciesColorPicker({
       colorId,
     });
 
+    // Ignore switching to the placeholder option. It shouldn't generally be
+    // doable once real options exist, and it doesn't represent a valid or
+    // meaningful transition in the case where it could happen.
+    if (newSpeciesId === "SpeciesColorPicker-species-loading-placeholder") {
+      return;
+    }
+
     const newSpecies = allSpecies.find((s) => s.id === newSpeciesId);
     if (!newSpecies) {
       // Trying to isolate Sentry issue IMPRESS-2020-1H, where an empty species
@@ -177,7 +191,7 @@ function SpeciesColorPicker({
     <Flex direction="row">
       <SpeciesColorSelect
         aria-label="Pet color"
-        value={colorId}
+        value={colorId || "SpeciesColorPicker-color-loading-placeholder"}
         // We also wait for the valid pairs before enabling, so users can't
         // trigger change events we're not ready for. Also, if the caller
         // hasn't provided species and color yet, assume it's still loading.
@@ -197,7 +211,9 @@ function SpeciesColorPicker({
           // placeholder. (Can happen during loading, or if an invalid color ID
           // like null is intentionally provided while the real value loads.)
           !visibleColors.some((c) => c.id === colorId) && (
-            <option>{colorPlaceholderText}</option>
+            <option value="SpeciesColorPicker-color-loading-placeholder">
+              {colorPlaceholderText}
+            </option>
           )
         }
         {
@@ -214,7 +230,7 @@ function SpeciesColorPicker({
       <Box width={size === "sm" ? 2 : 4} />
       <SpeciesColorSelect
         aria-label="Pet species"
-        value={speciesId}
+        value={speciesId || "SpeciesColorPicker-species-loading-placeholder"}
         // We also wait for the valid pairs before enabling, so users can't
         // trigger change events we're not ready for. Also, if the caller
         // hasn't provided species and color yet, assume it's still loading.
@@ -242,7 +258,9 @@ function SpeciesColorPicker({
           // ID like null is intentionally provided while the real value
           // loads.)
           !allSpecies.some((s) => s.id === speciesId) && (
-            <option>{speciesPlaceholderText}</option>
+            <option value="SpeciesColorPicker-species-loading-placeholder">
+              {speciesPlaceholderText}
+            </option>
           )
         }
         {