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

[Bug]: Args from Controls/Props are not passed properly to "Open Canvas in new tab" #20951

Closed
ozgurbaserdem opened this issue Feb 6, 2023 · 6 comments

Comments

@ozgurbaserdem
Copy link

ozgurbaserdem commented Feb 6, 2023

Describe the bug

So currently in our project we've had an issue when migrating to controls (from knobs). It all began with our toggled controls not getting passed properly when a component view was opened in a new window.

Our project is based on React, TypeScript and some components we use Material UI and customize them to match our design.

This is my "normal" storybook view, as you can see the isLoading control is toggled to true and it's shown correctly in this view:
Screenshot 2023-02-06 at 09 58 23

When I open the canvas in a new window, I get the args passed with me to the new tab (as you can see in the URL) but the isLoading state is not toggled properly:
Screenshot 2023-02-06 at 09 58 28

It is worth noting here that I believe this issue: #17517 is correct regarding which propTypes that work and which doesn't, because if I change the "label" control/prop in the story, which is of type: string, then it will get passed to the new canvas tab as well and work properly. So in this case type: string works, but not type: boolean.
Picture below where i've changed the label prop from "Datum" to "NEW DATE LABEL":
Screenshot 2023-02-06 at 10 25 56

I am now sure why this is happening and not sure how to reproduce this but as mentioned about it seems to be an issue with types from TypeScript.

Something worth mentioning is that we did find a work around for a little while by changing the reactDocgen from react-docgen-typescript to react-docgen. This did so our controls worked properly when opening canvas in new tab, but with that change a new issue emerged which was that the description field of the controls disappeared (as you can see in the image above where it says for instance "If loading is true the skeleton for DatePickers will be shown.", it instead showed "-").

So currently we have two choices:

  1. Use reactDocgen: react-docgen-typescript and get proper control/property descriptions but new canvas args doesn't work.
  2. reactDocgen: react-docgen and lose all control/property descriptions but the new canvas args work.

Obviously what we want is for both of these things to work simultaneously and if anyone has a work around I'd be happy to hear it. Or if it's just that we've set up our storybook wrong in some way I'd be happy if someone can see what's wrong.

This is our set up in main.js (I have tried adding compilerOptions and allowSyntheticDefaultImports esModuleInterop too, with no help https://storybook.js.org/docs/react/configure/typescript#overriding-the-configuration-to-infer-additional-props):
module.exports = { typescript: { check: false, checkOptions: {}, reactDocgen: "react-docgen-typescript", reactDocgenTypescriptOptions: { shouldExtractLiteralValuesFromEnum: true, propFilter: (prop) => { return prop.parent ? /@mui/.test(prop.parent.fileName) || !/node_modules/.test(prop.parent.fileName) : true; }, }, }, stories: ["../**/*.stories.mdx", "../**/*.stories.@(js|jsx|ts|tsx|mdx)"], webpackFinal: async (config) => { const fileLoaderRule = config.module.rules.find((rule) => rule.test.test(".svg") ); config.watchOptions = { ignored: [/node_modules([\\]+|\/)+(?!@sj-ab)/], }; fileLoaderRule.exclude = [pathToInlineSvg, pathToLogo]; config.module.rules.push({ test: /\.svg$/, include: [pathToInlineSvg, pathToLogo], use: [ { loader: "@svgr/webpack", options: { icon: true, svgProps: { focusable: "false", "aria-hidden": "true", }, }, }, "url-loader", ], }); return { ...config, resolve: { ...config.resolve, alias: { ...config.resolve.alias, "@emotion/core": toPath("node_modules/@emotion/react"), "emotion-theming": toPath("node_modules/@emotion/react"), }, }, }; }, addons: [ { name: "@storybook/addon-essentials", options: { actions: true, controls: true, }, }, "@storybook/addon-links", "@storybook/addon-a11y", "@storybook/addon-controls", "@storybook/addon-docs", "@feux/storybook-addon-bit", ], };

This is our DatePicker story:
export default { title: "Components/Date Picker", component: DatePicker, parameters: { controls: { exclude: ["rifmFormatter", "customAttribute", "staticPicker"] }, bit: { componentId: "ui/date-picker", } }, argTypes: { variant: { control: "select" }, hasWeekNumbers: { control: "boolean" }, disableFuture: { control: "boolean" }, disablePast: { control: "boolean" }, fullWidth: { control: "boolean" }, label: { control: "text", }, id: { control: "text" }, lang: { control: "select" }, minDate: { control: "text" }, maxDate: { control: "text" }, inputFormat: { control: "text" }, error: { control: "boolean" }, isLoading: { control: "boolean" }, customAttribute: { control: "text" }, errorMessage: { control: "text" }, customTranslation: { control: "object" }, value: { control: "text" }, helperText: { control: "text" }, disabled: { control: "boolean" }, onChange: { action: "onChange" }, }, args: { lang: "sv", variant: "automatic", label: "Datum", value: ${today}, minDate: undefined, maxDate: undefined, helperText: "Valfri hjälptext", inputFormat: "yyyy-MM-dd", errorMessage: "", error: false, disabled: false, isLoading: false, hasWeekNumbers: true, disableFuture: false, disablePast: false, fullWidth: false, }, }; export const DefaultDatePicker = ({ ...args }: DatePickerProps): React.ReactNode => { // useArgs sets dynamic values for controls. Similar to how we used useState. See https://medium.com/urbandataanalytics/changing-args-values-state-in-storybook-without-usestate-hook-15cc096716f7 const [{ value }, updateArgs] = useArgs(); const handleChange = (date: string | undefined) => { updateArgs({ value: date }); }; return <DatePicker {...args} value={value} onChange={handleChange} />; }; DefaultDatePicker.args = { variant: "automatic", minDate: ${today}, id: "basic-datepicker", };

This is our DatePicker types:
export interface DatePickerProps { /** ID for the input field inside the DatePicker, passed down through the renderInput. Make label and helperText accessible for screen readers. */ id: string; /** Language used for localization and date formatting. */ lang?: "sv" | "nb" | "en"; /** Translation object used for screen reader texts. */ customTranslation?: DeepPartial<Translation>; /** Visible label text. */ label: string; /** Optional helper text. */ helperText?: string; /** Earliest date to display in the format yyyy-MM-dd. */ minDate?: string; /** Latest date to display in the format yyyy-MM-dd. */ maxDate?: string; /** Callback to handle change of date. */ onChange: (date?: string) => void; /** Date value of the datepicker, in yyyy-MM-dd format. */ value?: string | null; /** If true, disables all date buttons after today's date LOCALLY. To make the disablbed dates follow swedish timezone make this false and pass today's date swedish time to maxDate instead. */ disableFuture?: boolean; /** If true, disables all date buttons before today's date LOCALLY. To make the disablbed dates follow swedish timezone make this false and pass today's date swedish time to minDate instead. */ disablePast?: boolean; /** If true, the inputfield will take up the parent elements full width. */ fullWidth?: boolean; /** If true, the picker and text field are disabled. */ disabled?: boolean; /** The inputformat we want to use for our date */ inputFormat?: string; /** If true, the picker shown will be the StaticDatePicker */ staticPicker?: boolean; /** Choose DatePicker variant, defaults to "automatic" */ variant?: "static" | "automatic" | "mobile" | "desktop"; /** If loading is true the skeleton for DatePickers will be shown. */ isLoading?: boolean; /** If the StaticDatePicker should draw focus immediately when rendered. Should be true when static datepicker is shown in popup or modal. If false, this prop will be set programmatically on first focus. */ autoFocus?: boolean; /** Set this to true the picker should show the week numbers next to dates. */ hasWeekNumbers?: boolean; /** Let's us display custom date format instead of value in the DatePicker TextField. Example: "2022-08-06" => "Tomorrow 6 August" */ rifmFormatter?: (string: string) => string; /** If true, the datepicker will be displayed in an error state. This is not mandatory and will override the default input validation error. */ error?: boolean; /** Custom error helper text message that will be displayed on error. This is not mandatory and will override the default input validation message. */ errorMessage?: string; /** Callback that is fired when input value returns a new validation error */ onError?: ( reason: "minDateError" | "invalidDateError" | "maxDateError", // The type of validation that failed value?: string // The date string that failed validation ) => void; /** Callback that is fired when input value passes validation */ onAccept?: ( value?: string // The date string that passed validation ) => void; /** Callback that is fired when text input loses focus */ onBlur?: (event?: React.FocusEvent<HTMLInputElement>) => void; }

To Reproduce

No response

System

Environment Info:

  System:
    OS: macOS 13.1
    CPU: (10) x64 Apple M1 Max
  Binaries:
    Node: 14.18.2 - ~/.nvm/versions/node/v14.18.2/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v14.18.2/bin/yarn
    npm: 6.14.15 - ~/.nvm/versions/node/v14.18.2/bin/npm
  Browsers:
    Chrome: 109.0.5414.119
    Edge: 109.0.1518.78
    Safari: 16.2

Additional context

Issues that might be connected:
#17517
#17413
#17902

@DanAnton
Copy link

DanAnton commented Sep 13, 2023

We are also experience issue in version 7.4.1 but in Angular. Does anyone has updates about this?

@teodorachiosa
Copy link

teodorachiosa commented Sep 15, 2023

It can be easily reproduced in a new project where storybook is added. When installing storybook, it comes with a story for a Button component. Versions tested: 7.0.8, 7.0.20, 7.4.1.
Change the default value for the Primary story in Button.stories.ts so that "primary" is false instead of true:

export const Primary: Story = {
  args: {
    primary: false,
    label: 'Button',
  },
};

This is the result:
When primary is set to true, the button turns blue. When opening in a new tab, the button is white.
storybook-issue

We've seen this happen for boolean and string values so far.

This issue is quite blocking for us as well, since we're trying to test the responsive behavior for each component in a new tab. The Viewport addon is too static for this purpose.

@Alxilin
Copy link

Alxilin commented Nov 8, 2023

+1 we have the same issue. However some boolean values are passed, and optional values are not, no matter even if i change them to free "text" input.
image

@EloiMgn
Copy link

EloiMgn commented Nov 8, 2023

It can be easily reproduced in a new project where storybook is added. When installing storybook, it comes with a story for a Button component. Versions tested: 7.0.8, 7.0.20, 7.4.1. Change the default value for the Primary story in Button.stories.ts so that "primary" is false instead of true:

export const Primary: Story = {
  args: {
    primary: false,
    label: 'Button',
  },
};

This is the result: When primary is set to true, the button turns blue. When opening in a new tab, the button is white. storybook-issue

We've seen this happen for boolean and string values so far.

This issue is quite blocking for us as well, since we're trying to test the responsive behavior for each component in a new tab. The Viewport addon is too static for this purpose.

Hello @elenachiosa I'm also facing this issue but only for boolean args since the upgrade to version 7 (i am currently in storybook 7.5.3). As a workaround i manually remove the "!" in front of the "true" in the new tab url but it is quite blocking when we need to test a specific implementation of our component and inspect the DOM without all of Storybook DOM interfering.

@MichaelAllenWarner
Copy link

I see the same behavior @EloiMgn describes for boolean args. I have to manually remove exclamation marks from URLs before sharing them with colleagues.

@shilman
Copy link
Member

shilman commented Dec 25, 2023

closing as dupe to #25035

@shilman shilman closed this as not planned Won't fix, can't repro, duplicate, stale Dec 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

7 participants