Merge pull request #11704 from jonspalmer/controls_update_name_not_key

Addon-controls: Fix update logic for argTypes with custom names
storybookjs · Jul 29, 2020 · 2ffe3ae · 2ffe3ae
2 parents 820dddd + 28b64b6
commit 2ffe3ae
Show file tree

Hide file tree

Showing 24 changed files with 62 additions and 49 deletions.
diff --git 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' },
   },
 };
 

diff --git 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,20 +32,22 @@ 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), []);
   const onFocus = useCallback(() => setFocused(true), []);
 
   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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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>