From 111d5f31b1a4d058fca6ac82856b2ba6bd770cbc Mon Sep 17 00:00:00 2001
From: Jon Palmer <328224+jonspalmer@users.noreply.github.com>
Date: Mon, 27 Jul 2020 20:49:23 -0400
Subject: [PATCH 1/2] Failing test case for Controls with names not updating

---
 .../official-storybook/stories/addon-controls.stories.tsx   | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/examples/official-storybook/stories/addon-controls.stories.tsx b/examples/official-storybook/stories/addon-controls.stories.tsx
index 132725d71d52..b6537a686ad4 100644
--- a/examples/official-storybook/stories/addon-controls.stories.tsx
+++ b/examples/official-storybook/stories/addon-controls.stories.tsx
@@ -5,9 +5,9 @@ export default {
   title: 'Addons/Controls',
   component: Button,
   argTypes: {
-    children: { control: 'text' },
-    type: { control: 'text' },
-    somethingElse: { control: 'object' },
+    children: { control: 'text', name: 'Children' },
+    type: { control: 'text', name: 'Type' },
+    somethingElse: { control: 'object', name: 'Something Else' },
   },
 };
 

From 28b64b62dafff6f1fccd571c6a21e9ad5f56dfa5 Mon Sep 17 00:00:00 2001
From: Jon Palmer <328224+jonspalmer@users.noreply.github.com>
Date: Wed, 29 Jul 2020 09:50:22 -0400
Subject: [PATCH 2/2] simpler change

---
 .../src/blocks/ArgsTable/ArgControl.tsx       | 12 ++++---
 .../src/blocks/ArgsTable/ArgRow.stories.tsx   | 35 ++++++++++++-------
 lib/components/src/controls/Array.stories.tsx |  2 +-
 lib/components/src/controls/Array.tsx         |  2 +-
 .../src/controls/Boolean.stories.tsx          |  2 +-
 lib/components/src/controls/Boolean.tsx       |  2 +-
 lib/components/src/controls/Color.stories.tsx |  2 +-
 lib/components/src/controls/Color.tsx         |  2 +-
 lib/components/src/controls/Date.stories.tsx  |  4 +--
 lib/components/src/controls/Date.tsx          |  4 +--
 .../src/controls/Number.stories.tsx           |  4 +--
 lib/components/src/controls/Number.tsx        |  2 +-
 .../src/controls/Object.stories.tsx           |  4 +--
 lib/components/src/controls/Object.tsx        |  2 +-
 lib/components/src/controls/Range.stories.tsx |  2 +-
 lib/components/src/controls/Range.tsx         |  2 +-
 lib/components/src/controls/Text.stories.tsx  |  2 +-
 lib/components/src/controls/Text.tsx          |  2 +-
 .../src/controls/options/Checkbox.tsx         |  6 ++--
 .../src/controls/options/Options.stories.tsx  |  2 +-
 lib/components/src/controls/options/Radio.tsx |  4 +--
 .../src/controls/options/Select.tsx           |  4 +--
 lib/components/src/controls/types.ts          |  2 +-
 23 files changed, 59 insertions(+), 46 deletions(-)

diff --git a/lib/components/src/blocks/ArgsTable/ArgControl.tsx b/lib/components/src/blocks/ArgsTable/ArgControl.tsx
index d8a61cc1edb2..27fda9531874 100644
--- a/lib/components/src/blocks/ArgsTable/ArgControl.tsx
+++ b/lib/components/src/blocks/ArgsTable/ArgControl.tsx
@@ -21,7 +21,7 @@ export interface ArgControlProps {
 const NoControl = () => <>-</>;
 
 export const ArgControl: FC<ArgControlProps> = ({ row, arg, updateArgs }) => {
-  const { name, control } = row;
+  const { key, control } = row;
 
   const [isFocused, setFocused] = useState(false);
   // box because arg can be a fn (e.g. actions) and useState calls fn's
@@ -32,12 +32,12 @@ export const ArgControl: FC<ArgControlProps> = ({ row, arg, updateArgs }) => {
   }, [isFocused, arg]);
 
   const onChange = useCallback(
-    (argName: string, argVal: any) => {
+    (argVal: any) => {
       setBoxedValue({ value: argVal });
-      updateArgs({ [name]: argVal });
+      updateArgs({ [key]: argVal });
       return argVal;
     },
-    [updateArgs, name]
+    [updateArgs, key]
   );
 
   const onBlur = useCallback(() => setFocused(false), []);
@@ -45,7 +45,9 @@ export const ArgControl: FC<ArgControlProps> = ({ row, arg, updateArgs }) => {
 
   if (!control || control.disable) return <NoControl />;
 
-  const props = { name, argType: row, value: boxedValue.value, onChange, onBlur, onFocus };
+  // row.name is a display name and not a suitable DOM input id or name - i might contain whitespace etc.
+  // row.key is a hash key and therefore a much safer choice
+  const props = { name: key, argType: row, value: boxedValue.value, onChange, onBlur, onFocus };
   switch (control.type) {
     case 'array':
       return <ArrayControl {...props} {...control} />;
diff --git a/lib/components/src/blocks/ArgsTable/ArgRow.stories.tsx b/lib/components/src/blocks/ArgsTable/ArgRow.stories.tsx
index 84f9239ce3a1..602d79c02093 100644
--- a/lib/components/src/blocks/ArgsTable/ArgRow.stories.tsx
+++ b/lib/components/src/blocks/ArgsTable/ArgRow.stories.tsx
@@ -28,7 +28,8 @@ export const String = Template.bind({});
 String.args = {
   ...baseArgs,
   row: {
-    name: 'someString',
+    key: 'someString',
+    name: 'Some String',
     description: 'someString description',
     type: { required: true },
     control: { type: 'text' },
@@ -44,7 +45,7 @@ LongName.args = {
   ...baseArgs,
   row: {
     ...String.args.row,
-    name: 'reallyLongStringThatTakesUpSpace',
+    name: 'Really Long String That Takes Up Space',
   },
 };
 
@@ -61,7 +62,8 @@ export const Boolean = Template.bind({});
 Boolean.args = {
   ...baseArgs,
   row: {
-    name: 'someBoolean',
+    key: 'someBoolean',
+    name: 'Some Boolean',
     description: 'someBoolean description',
     type: { required: true },
     control: { type: 'boolean' },
@@ -76,7 +78,8 @@ export const Color = Template.bind({});
 Color.args = {
   ...baseArgs,
   row: {
-    name: 'someColor',
+    key: 'someColor',
+    name: 'Some Color',
     type: { name: 'string' },
     description: 'someColor description',
     defaultValue: '#ff0',
@@ -88,7 +91,8 @@ export const Date = Template.bind({});
 Date.args = {
   ...baseArgs,
   row: {
-    name: 'someDate',
+    key: 'someDate',
+    name: 'Some Date',
     type: { name: 'string' },
     description: 'someDate description',
     control: { type: 'date' },
@@ -99,7 +103,8 @@ export const Number = Template.bind({});
 Number.args = {
   ...baseArgs,
   row: {
-    name: 'someNumber',
+    key: 'someNumber',
+    name: 'Some Number',
     description: 'someNumber description',
     type: { required: false },
     table: {
@@ -123,7 +128,8 @@ export const Radio = Template.bind({});
 Radio.args = {
   ...baseArgs,
   row: {
-    name: 'someEnum',
+    key: 'someEnum',
+    name: 'Some Enum',
     description: 'someEnum description',
     control: { type: 'radio', options: ['a', 'b', 'c'] },
   },
@@ -178,7 +184,8 @@ export const ObjectOf = Template.bind({});
 ObjectOf.args = {
   ...baseArgs,
   row: {
-    name: 'someObject',
+    key: 'someObject',
+    name: 'Some Object',
     description: 'A simple `objectOf` propType.',
     table: {
       type: { summary: 'objectOf(number)' },
@@ -192,7 +199,8 @@ export const ArrayOf = Template.bind({});
 ArrayOf.args = {
   ...baseArgs,
   row: {
-    name: 'someArray',
+    key: 'someArray',
+    name: 'Some Array',
     description: 'array of a certain type',
     table: {
       type: { summary: 'number[]' },
@@ -206,7 +214,8 @@ export const ComplexObject = Template.bind({});
 ComplexObject.args = {
   ...baseArgs,
   row: {
-    name: 'someComplex',
+    key: 'someComplex',
+    name: 'Some Complex',
     description: 'A very complex `objectOf` propType.',
     table: {
       type: {
@@ -234,7 +243,8 @@ export const Func = Template.bind({});
 Func.args = {
   ...baseArgs,
   row: {
-    name: 'concat',
+    key: 'concat',
+    name: 'Concat',
     description: 'concat 2 string values.',
     type: { required: true },
     table: {
@@ -256,7 +266,8 @@ export const Markdown = Template.bind({});
 Markdown.args = {
   ...baseArgs,
   row: {
-    name: 'someString',
+    key: 'someString',
+    name: 'Some String',
     description:
       'A `prop` can *support* __markdown__ syntax. This was ship in ~~5.2~~ 5.3. [Find more info in the storybook docs.](https://storybook.js.org/)',
     table: {
diff --git a/lib/components/src/controls/Array.stories.tsx b/lib/components/src/controls/Array.stories.tsx
index 9feffc7da951..5b75557911e5 100644
--- a/lib/components/src/controls/Array.stories.tsx
+++ b/lib/components/src/controls/Array.stories.tsx
@@ -13,7 +13,7 @@ const Template = (initialValue: any) => {
       <ArrayControl
         name="array"
         value={value}
-        onChange={(name, newVal) => setValue(newVal)}
+        onChange={(newVal) => setValue(newVal)}
         separator=","
       />
       <ul>{value && value.map((item) => <li key={item}>{item}</li>)}</ul>
diff --git a/lib/components/src/controls/Array.tsx b/lib/components/src/controls/Array.tsx
index b43c9109aca5..4694528f5d1b 100644
--- a/lib/components/src/controls/Array.tsx
+++ b/lib/components/src/controls/Array.tsx
@@ -27,7 +27,7 @@ export const ArrayControl: FC<ArrayProps> = ({
   const handleChange = useCallback(
     (e: ChangeEvent<HTMLTextAreaElement>): void => {
       const { value: newVal } = e.target;
-      onChange(name, parse(newVal, separator));
+      onChange(parse(newVal, separator));
     },
     [onChange]
   );
diff --git a/lib/components/src/controls/Boolean.stories.tsx b/lib/components/src/controls/Boolean.stories.tsx
index 0d9050aac9e2..b604fec68bf0 100644
--- a/lib/components/src/controls/Boolean.stories.tsx
+++ b/lib/components/src/controls/Boolean.stories.tsx
@@ -10,7 +10,7 @@ const Template = (initialValue?: boolean) => {
   const [value, setValue] = useState(initialValue);
   return (
     <>
-      <BooleanControl name="boolean" value={value} onChange={(name, newVal) => setValue(newVal)} />
+      <BooleanControl name="boolean" value={value} onChange={(newVal) => setValue(newVal)} />
       <p>value: {typeof value === 'boolean' ? value.toString() : value}</p>
     </>
   );
diff --git a/lib/components/src/controls/Boolean.tsx b/lib/components/src/controls/Boolean.tsx
index 396a93292fa6..3ad6f03f3f85 100644
--- a/lib/components/src/controls/Boolean.tsx
+++ b/lib/components/src/controls/Boolean.tsx
@@ -84,7 +84,7 @@ export const BooleanControl: FC<BooleanProps> = ({ name, value, onChange, onBlur
     <input
       id={name}
       type="checkbox"
-      onChange={(e) => onChange(name, e.target.checked)}
+      onChange={(e) => onChange(e.target.checked)}
       checked={value}
       {...{ name, onBlur, onFocus }}
     />
diff --git a/lib/components/src/controls/Color.stories.tsx b/lib/components/src/controls/Color.stories.tsx
index 59bda246bd90..3f3cf8455017 100644
--- a/lib/components/src/controls/Color.stories.tsx
+++ b/lib/components/src/controls/Color.stories.tsx
@@ -12,7 +12,7 @@ const Template = (initialValue?: string, presetColors?: string[]) => {
     <ColorControl
       name="Color"
       value={value}
-      onChange={(name, newVal) => setValue(newVal)}
+      onChange={(newVal) => setValue(newVal)}
       presetColors={presetColors}
     />
   );
diff --git a/lib/components/src/controls/Color.tsx b/lib/components/src/controls/Color.tsx
index 6fa2a2d0c6ec..6d4a460ef45a 100644
--- a/lib/components/src/controls/Color.tsx
+++ b/lib/components/src/controls/Color.tsx
@@ -66,7 +66,7 @@ export const ColorControl: FC<ColorProps> = ({
         >
           <SketchPicker
             color={value}
-            onChange={(color: ColorResult) => onChange(name, format(color))}
+            onChange={(color: ColorResult) => onChange(format(color))}
             {...{ onFocus, onBlur, presetColors }}
           />
         </Popover>
diff --git a/lib/components/src/controls/Date.stories.tsx b/lib/components/src/controls/Date.stories.tsx
index 165464d02696..761c9a54c42f 100644
--- a/lib/components/src/controls/Date.stories.tsx
+++ b/lib/components/src/controls/Date.stories.tsx
@@ -10,7 +10,7 @@ export const Basic = () => {
   const [value, setValue] = useState(new Date(2020, 4, 20));
   return (
     <>
-      <DateControl name="date" value={value} onChange={(name, newVal) => setValue(newVal)} />
+      <DateControl name="date" value={value} onChange={(newVal) => setValue(newVal)} />
       <p>{value && new Date(value).toISOString()}</p>
     </>
   );
@@ -20,7 +20,7 @@ export const Undefined = () => {
   const [value, setValue] = useState(undefined);
   return (
     <>
-      <DateControl name="date" value={value} onChange={(name, newVal) => setValue(newVal)} />
+      <DateControl name="date" value={value} onChange={(newVal) => setValue(newVal)} />
       <p>{value && new Date(value).toISOString()}</p>
     </>
   );
diff --git a/lib/components/src/controls/Date.tsx b/lib/components/src/controls/Date.tsx
index d90175c712e3..99387d9a298a 100644
--- a/lib/components/src/controls/Date.tsx
+++ b/lib/components/src/controls/Date.tsx
@@ -79,7 +79,7 @@ export const DateControl: FC<DateProps> = ({ name, value, onChange, onFocus, onB
     result.setMonth(parsed.getMonth());
     result.setDate(parsed.getDate());
     const time = result.getTime();
-    if (time) onChange(name, time);
+    if (time) onChange(time);
     setValid(!!time);
   };
 
@@ -89,7 +89,7 @@ export const DateControl: FC<DateProps> = ({ name, value, onChange, onFocus, onB
     result.setHours(parsed.getHours());
     result.setMinutes(parsed.getMinutes());
     const time = result.getTime();
-    if (time) onChange(name, time);
+    if (time) onChange(time);
     setValid(!!time);
   };
 
diff --git a/lib/components/src/controls/Number.stories.tsx b/lib/components/src/controls/Number.stories.tsx
index b58f47868ccc..de6b0459e904 100644
--- a/lib/components/src/controls/Number.stories.tsx
+++ b/lib/components/src/controls/Number.stories.tsx
@@ -10,7 +10,7 @@ export const Basic = () => {
   const [value, setValue] = useState(10);
   return (
     <>
-      <NumberControl name="number" value={value} onChange={(name, newVal) => setValue(newVal)} />
+      <NumberControl name="number" value={value} onChange={(newVal) => setValue(newVal)} />
       <p>{value}</p>
     </>
   );
@@ -20,7 +20,7 @@ export const Undefined = () => {
   const [value, setValue] = useState(undefined);
   return (
     <>
-      <NumberControl name="number" value={value} onChange={(name, newVal) => setValue(newVal)} />
+      <NumberControl name="number" value={value} onChange={(newVal) => setValue(newVal)} />
       <p>{value}</p>
     </>
   );
diff --git a/lib/components/src/controls/Number.tsx b/lib/components/src/controls/Number.tsx
index af30e7f82190..fcfe43efe386 100644
--- a/lib/components/src/controls/Number.tsx
+++ b/lib/components/src/controls/Number.tsx
@@ -28,7 +28,7 @@ export const NumberControl: FC<NumberProps> = ({
   onFocus,
 }) => {
   const handleChange = (event: ChangeEvent<HTMLInputElement>) => {
-    onChange(name, parse(event.target.value));
+    onChange(parse(event.target.value));
   };
 
   return (
diff --git a/lib/components/src/controls/Object.stories.tsx b/lib/components/src/controls/Object.stories.tsx
index 3b43e8528f6e..3bf1a784ec63 100644
--- a/lib/components/src/controls/Object.stories.tsx
+++ b/lib/components/src/controls/Object.stories.tsx
@@ -10,7 +10,7 @@ const Template = (initialValue: any) => {
   const [value, setValue] = useState(initialValue);
   return (
     <>
-      <ObjectControl name="object" value={value} onChange={(name, newVal) => setValue(newVal)} />
+      <ObjectControl name="object" value={value} onChange={(newVal) => setValue(newVal)} />
       <p>{value && JSON.stringify(value)}</p>
     </>
   );
@@ -30,7 +30,7 @@ export const ValidatedAsArray = () => {
         name="object"
         argType={{ type: { name: 'array' } }}
         value={value}
-        onChange={(name, newVal) => setValue(newVal)}
+        onChange={(newVal) => setValue(newVal)}
       />
       <p>{value && JSON.stringify(value)}</p>
     </>
diff --git a/lib/components/src/controls/Object.tsx b/lib/components/src/controls/Object.tsx
index e39d7ad1364c..84287569f714 100644
--- a/lib/components/src/controls/Object.tsx
+++ b/lib/components/src/controls/Object.tsx
@@ -47,7 +47,7 @@ export const ObjectControl: FC<ObjectProps> = ({
         const newVal = parse(e.target.value);
         const newValid = validate(newVal, argType);
         if (newValid && !deepEqual(value, newVal)) {
-          onChange(name, newVal);
+          onChange(newVal);
         }
         setValid(newValid);
       } catch (err) {
diff --git a/lib/components/src/controls/Range.stories.tsx b/lib/components/src/controls/Range.stories.tsx
index a2214ef7b2ed..e06531f2fab6 100644
--- a/lib/components/src/controls/Range.stories.tsx
+++ b/lib/components/src/controls/Range.stories.tsx
@@ -13,7 +13,7 @@ const Template = (initialValue?: number) => {
       <RangeControl
         name="range"
         value={value}
-        onChange={(name, newVal) => setValue(newVal)}
+        onChange={(newVal) => setValue(newVal)}
         min={0}
         max={20}
         step={2}
diff --git a/lib/components/src/controls/Range.tsx b/lib/components/src/controls/Range.tsx
index 2abee0d063b9..09d00a378e52 100644
--- a/lib/components/src/controls/Range.tsx
+++ b/lib/components/src/controls/Range.tsx
@@ -162,7 +162,7 @@ export const RangeControl: FC<RangeProps> = ({
   onFocus,
 }) => {
   const handleChange = (event: ChangeEvent<HTMLInputElement>) => {
-    onChange(name, parse(event.target.value));
+    onChange(parse(event.target.value));
   };
   return (
     <RangeWrapper>
diff --git a/lib/components/src/controls/Text.stories.tsx b/lib/components/src/controls/Text.stories.tsx
index 2f8007ec5c41..da2bb52a72a5 100644
--- a/lib/components/src/controls/Text.stories.tsx
+++ b/lib/components/src/controls/Text.stories.tsx
@@ -10,7 +10,7 @@ const Template = (initialValue?: string) => {
   const [value, setValue] = useState(initialValue);
   return (
     <>
-      <TextControl name="Text" value={value} onChange={(name, newVal) => setValue(newVal)} />
+      <TextControl name="Text" value={value} onChange={(newVal) => setValue(newVal)} />
       <p>{value}</p>
     </>
   );
diff --git a/lib/components/src/controls/Text.tsx b/lib/components/src/controls/Text.tsx
index 5dd0fd804a44..edf6e53c0b2b 100644
--- a/lib/components/src/controls/Text.tsx
+++ b/lib/components/src/controls/Text.tsx
@@ -14,7 +14,7 @@ const format = (value?: TextValue) => value || '';
 
 export const TextControl: FC<TextProps> = ({ name, value, onChange, onFocus, onBlur }) => {
   const handleChange = (event: ChangeEvent<HTMLTextAreaElement>) => {
-    onChange(name, event.target.value);
+    onChange(event.target.value);
   };
   return (
     <Wrapper>
diff --git a/lib/components/src/controls/options/Checkbox.tsx b/lib/components/src/controls/options/Checkbox.tsx
index 95b77aeb73aa..5695e1b9a257 100644
--- a/lib/components/src/controls/options/Checkbox.tsx
+++ b/lib/components/src/controls/options/Checkbox.tsx
@@ -59,20 +59,20 @@ export const CheckboxControl: FC<CheckboxProps> = ({
     } else {
       updated.push(option);
     }
-    onChange(name, selectedValues(updated, options));
+    onChange(selectedValues(updated, options));
     setSelected(updated);
   };
 
   return (
     <Wrapper isInline={isInline}>
-      {Object.keys(options).map((key: string) => {
+      {Object.keys(options).map((key) => {
         const id = `${name}-${key}`;
         return (
           <Label key={id} htmlFor={id}>
             <input
               type="checkbox"
               id={id}
-              name={name}
+              name={id}
               value={key}
               onChange={handleChange}
               checked={selected?.includes(key)}
diff --git a/lib/components/src/controls/options/Options.stories.tsx b/lib/components/src/controls/options/Options.stories.tsx
index 5c70774b04db..9a2066620230 100644
--- a/lib/components/src/controls/options/Options.stories.tsx
+++ b/lib/components/src/controls/options/Options.stories.tsx
@@ -23,7 +23,7 @@ const optionsHelper = (options, type, isMulti) => {
         options={options}
         value={value}
         type={type}
-        onChange={(_name, newVal) => setValue(newVal)}
+        onChange={(newVal) => setValue(newVal)}
       />
       {value && Array.isArray(value) ? (
         // eslint-disable-next-line react/no-array-index-key
diff --git a/lib/components/src/controls/options/Radio.tsx b/lib/components/src/controls/options/Radio.tsx
index 5830d49e2b68..0078c6c43d49 100644
--- a/lib/components/src/controls/options/Radio.tsx
+++ b/lib/components/src/controls/options/Radio.tsx
@@ -58,9 +58,9 @@ export const RadioControl: FC<RadioProps> = ({ name, options, value, onChange, i
             <input
               type="radio"
               id={id}
-              name={name}
+              name={id}
               value={key}
-              onChange={(e) => onChange(name, options[e.currentTarget.value])}
+              onChange={(e) => onChange(options[e.currentTarget.value])}
               checked={key === selection}
             />
             <Text>{key}</Text>
diff --git a/lib/components/src/controls/options/Select.tsx b/lib/components/src/controls/options/Select.tsx
index 661e37e2ca2d..bfaa91ef8d88 100644
--- a/lib/components/src/controls/options/Select.tsx
+++ b/lib/components/src/controls/options/Select.tsx
@@ -89,7 +89,7 @@ const NO_SELECTION = 'Select...';
 
 const SingleSelect: FC<SelectProps> = ({ name, value, options, onChange }) => {
   const handleChange = (e: ChangeEvent<HTMLSelectElement>) => {
-    onChange(name, options[e.currentTarget.value]);
+    onChange(options[e.currentTarget.value]);
   };
   const selection = selectedKey(value, options) || NO_SELECTION;
 
@@ -113,7 +113,7 @@ const MultiSelect: FC<SelectProps> = ({ name, value, options, onChange }) => {
     const selection = Array.from(e.currentTarget.options)
       .filter((option) => option.selected)
       .map((option) => option.value);
-    onChange(name, selectedValues(selection, options));
+    onChange(selectedValues(selection, options));
   };
   const selection = selectedKeys(value, options);
 
diff --git a/lib/components/src/controls/types.ts b/lib/components/src/controls/types.ts
index b8c737823d37..20bf32df2a87 100644
--- a/lib/components/src/controls/types.ts
+++ b/lib/components/src/controls/types.ts
@@ -6,7 +6,7 @@ export interface ControlProps<T> {
   value?: T;
   defaultValue?: T;
   argType?: ArgType;
-  onChange: (name: string, value: T) => T | void;
+  onChange: (value: T) => T | void;
   onFocus?: (evt: any) => void;
   onBlur?: (evt: any) => void;
 }