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-docs: props with default values appear before props without default values in the props table #8539

Closed
patricklafrance opened this issue Oct 23, 2019 · 12 comments

Comments

@patricklafrance
Copy link
Member

Describe the bug
If a prop have a default value, it will be displayed in the props tables before a prop without a default value.

Expected behavior
All props should appear in the same order as they are defined, no matter if they have a default value or not.

An easy way to fix this, would be to sort the props alphabetically. IMO it would also make the documentation easier to read.

Screenshots

For the following prop types:

static propTypes = {
        /**
         * A controlled start date value.
         */
        startDate: momentType,
        reactDatesCalendar: node,
};

The following is rendered:

image

@Joseph-Whiunui
Copy link

Hi, would I be able to grab this issue?

@shilman
Copy link
Member

shilman commented Oct 24, 2019

Thanks @Joseph-Whiunui! All yours

@Joseph-Whiunui
Copy link

Hi, have I taken too long and this issue been passed on?

I had added a simple sort function rows.sort((a,b) => (a.name > b.name) ? 1 : ((b.name > a.name) ? -1 : 0)); to PropsTable.tsx, but I am having trouble getting yarn lint and yarn test --core to run.

Should I continue trying to get those to run, maybe ask in the discord, or leave it to Patrick?

@patricklafrance
Copy link
Member Author

No go ahead @Joseph-Whiunui I have not look at it yet there is many other issues I can focus on!

@patricklafrance
Copy link
Member Author

patricklafrance commented Nov 12, 2019

@Joseph-Whiunui how is it going with this issue? Would be nice to include this fix in the next beta (which should be this week!)

@Joseph-Whiunui
Copy link

@patricklafrance Sorry there hasn't been much progress. I still can't get yarn test to run and didn't manage to find any help on the discord. I think it's a windows issue, but I have no idea how to fix it.

If you want to get this change out, I can bow out and let you do it. Didn't want to give up haha but I'm pretty stuck.

@patricklafrance
Copy link
Member Author

I almost have this problem the trick is to start the tests with the --testPathPattern argument.

yarn jest --testPathPattern=myfile --watch

@Joseph-Whiunui
Copy link

Hi @patricklafrance, thanks for the help! I started fresh just to be sure and used your command and got the tests running.

I've made a new PR, it is currently failing but I have asked in the discord for next steps.

@shilman
Copy link
Member

shilman commented Nov 18, 2019

Ooh-la-la!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.0-beta.1 containing PR #8857 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

@shilman
Copy link
Member

shilman commented Nov 19, 2019

Son of a gun!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.0-beta.2 containing PR #8867 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

@chadlavi-casebook
Copy link

I still experience this bug on version 6.1.18

@shilman
Copy link
Member

shilman commented Feb 25, 2021

will address in #9917

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants