Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addon-controls: Fix update logic for argTypes with custom names #11704

Merged
merged 2 commits into from
Jul 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
},
};

Expand Down
12 changes: 7 additions & 5 deletions lib/components/src/blocks/ArgsTable/ArgControl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -32,20 +32,22 @@ export const ArgControl: FC<ArgControlProps> = ({ row, arg, updateArgs }) => {
}, [isFocused, arg]);

const onChange = useCallback(
(argName: string, argVal: any) => {
(argVal: any) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any reason for the callback to have the key. Before we were ignoring it anyway. The Controls shouldn't need to know this detail.

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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shilman and I discussed adding a key prop (which would actually have to be argKey as key is a React reserved property). However, with the onChange adjustment its mostly unnecessary. We're really intending to hand in something that can be used as a DOM Input id/name. This makes for a much smaller change

const props = { name: key, argType: row, value: boxedValue.value, onChange, onBlur, onFocus };
switch (control.type) {
case 'array':
return <ArrayControl {...props} {...control} />;
Expand Down
35 changes: 23 additions & 12 deletions lib/components/src/blocks/ArgsTable/ArgRow.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
Expand All @@ -44,7 +45,7 @@ LongName.args = {
...baseArgs,
row: {
...String.args.row,
name: 'reallyLongStringThatTakesUpSpace',
name: 'Really Long String That Takes Up Space',
},
};

Expand All @@ -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' },
Expand All @@ -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',
Expand All @@ -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' },
Expand All @@ -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: {
Expand All @@ -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'] },
},
Expand Down Expand Up @@ -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)' },
Expand All @@ -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[]' },
Expand All @@ -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: {
Expand Down Expand Up @@ -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: {
Expand All @@ -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: {
Expand Down
2 changes: 1 addition & 1 deletion lib/components/src/controls/Array.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion lib/components/src/controls/Array.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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]
);
Expand Down
2 changes: 1 addition & 1 deletion lib/components/src/controls/Boolean.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
</>
);
Expand Down
2 changes: 1 addition & 1 deletion lib/components/src/controls/Boolean.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
/>
Expand Down
2 changes: 1 addition & 1 deletion lib/components/src/controls/Color.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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}
/>
);
Expand Down
2 changes: 1 addition & 1 deletion lib/components/src/controls/Color.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
4 changes: 2 additions & 2 deletions lib/components/src/controls/Date.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
</>
);
Expand All @@ -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>
</>
);
Expand Down
4 changes: 2 additions & 2 deletions lib/components/src/controls/Date.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};

Expand All @@ -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);
};

Expand Down
4 changes: 2 additions & 2 deletions lib/components/src/controls/Number.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
</>
);
Expand All @@ -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>
</>
);
Expand Down
2 changes: 1 addition & 1 deletion lib/components/src/controls/Number.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
4 changes: 2 additions & 2 deletions lib/components/src/controls/Object.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
</>
);
Expand All @@ -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>
</>
Expand Down
2 changes: 1 addition & 1 deletion lib/components/src/controls/Object.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion lib/components/src/controls/Range.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
2 changes: 1 addition & 1 deletion lib/components/src/controls/Range.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion lib/components/src/controls/Text.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
</>
);
Expand Down
2 changes: 1 addition & 1 deletion lib/components/src/controls/Text.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
6 changes: 3 additions & 3 deletions lib/components/src/controls/options/Checkbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
Expand Down
2 changes: 1 addition & 1 deletion lib/components/src/controls/options/Options.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/components/src/controls/options/Radio.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Loading