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.

Sign up for GitHub

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

Jump to bottom

Controls: Reset ArgsTable state when switching stories #14493

Merged
merged 2 commits into from Apr 7, 2021
Merged

Controls: Reset ArgsTable state when switching stories #14493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
53c9df2
Ensure ArgsTable state is reset when switching stories
ghengeveld Apr 6, 2021
0e73dc3
Add missing useCallback deps
ghengeveld Apr 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion addons/controls/src/ControlsPanel.tsx
View file
Open in desktop
@@ -1,5 +1,5 @@
import React, { FC } from 'react';
import { ArgTypes, useArgs, useArgTypes, useParameter } from '@storybook/api';
import { ArgTypes, useArgs, useArgTypes, useParameter, useStorybookState } from '@storybook/api';
import { ArgsTable, NoControlsWarning, PresetColor, SortType } from '@storybook/components';

import { PARAM_KEY } from './constants';
Expand All @@ -21,6 +21,7 @@ export const ControlsPanel: FC = () => {
presetColors,
hideNoControlsWarning = false,
} = useParameter<ControlsParameters>(PARAM_KEY, {});
const { path } = useStorybookState();

const hasControls = Object.values(rows).some((arg) => arg?.control);
const showWarning = !(hasControls && isArgsStory) && !hideNoControlsWarning;
Expand All @@ -36,6 +37,7 @@ export const ControlsPanel: FC = () => {
{showWarning && <NoControlsWarning />}
<ArgsTable
{...{
key: path, // resets state when switching stories
Copy link
Member

Choose a reason for hiding this comment

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

Would storyId make more sense here?

Copy link
Member Author

@ghengeveld ghengeveld Apr 7, 2021 •
edited

Choose a reason for hiding this comment

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

Yeah I did that at first but it would then have to be:

{
  key: refId ? `${refId}_${storyId}` : storyId
}

path achieves the same thing but without the added complexity of dealing with refId (it's in the path already).

compact: !expanded && hasControls,
rows: withPresetColors,
args,
Expand Down
21 changes: 12 additions & 9 deletions lib/components/src/controls/Color.tsx
View file
Open in desktop
Expand Up @@ -218,14 +218,17 @@ const useColorInput = (initialValue: string, onChange: (value: string) => string
colorSpace,
]);

const updateValue = useCallback((update: string) => {
const parsed = parseValue(update);
setValue(parsed?.value || update || '');
if (!parsed) return;
setColor(parsed);
setColorSpace(parsed.colorSpace);
onChange(parsed.value);
}, []);
const updateValue = useCallback(
(update: string) => {
const parsed = parseValue(update);
setValue(parsed?.value || update || '');
if (!parsed) return;
setColor(parsed);
setColorSpace(parsed.colorSpace);
onChange(parsed.value);
},
[onChange]
);

const cycleColorSpace = useCallback(() => {
let next = COLOR_SPACES.indexOf(colorSpace) + 1;
Expand All @@ -234,7 +237,7 @@ const useColorInput = (initialValue: string, onChange: (value: string) => string
const update = color?.[COLOR_SPACES[next]] || '';
setValue(update);
onChange(update);
}, [color, colorSpace]);
}, [color, colorSpace, onChange]);

return { value, realValue, updateValue, color, colorSpace, cycleColorSpace };
};
Expand Down