Addon-docs: Expand/collapse prop table entries by their 'parent'

[atanasster]

the typescript prop tables can often times get very long with components that are accepting props from many interfaces. Example:

export type IDropInputProps = IDropInputOwnProps & TextInputProps & JSX.IntrinsicElements['input'];

the prop table in this case is really long:
https://atanasster.github.io/grommet-controls/?path=/docs/controls-input-dropinput--main

The example prop table json file:
https://github.com/atanasster/grommet-controls/blob/master/src/components/DropInput/doc/DropInput.json

A screenshot how the parent interface name can be collapsed. By default suggestion is to have expanded only the first interface name:

[shilman]

Great idea. I think there are a few aspects to this:

• UI design cc @domyen & implementation
• Heuristics for when to collapse automatically and/or configurations
• Hooks (a la Addon-docs: Proposal to link interfaces #7944 (comment)) for fine-grained control

[atanasster]

this thread might be interesting https://stackoverflow.com/questions/5859561/getting-the-closest-string-match .

[domyen]

I can see how this would be necessary for composite components.

Question:

• @atanasster is there a way to sort/group the props by parent interface currently?

Some strawman UI ideas:

• Group by parent interface when possible (MVP)
• Expand all if total prop count is less than 20
• Collapse parent interfaces when child prop count is greater than 5 (or whatever number we decide).

[atanasster]

@domyen very good points.

• Currently the parent interface name is not displayed and not sortable (and it only makes sense in typescript I believe, so it might not be something to always have visible)
• do you have a collapse/expand component or UI pattern that you use?

[shilman]

Sort of related: In non-react frameworks there are more things to put in tables than just props. For example, Vue has slots & events: #6663

Also sort of related: Multiple components / sub-components: #7811

[atanasster]

@domyen - do you have some animated expand/collapse (ie https://github.com/Stanko/react-animate-height) in sb.components or sds?
Is it ok to introduce dependency on react-animate-height ?

[stale]

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

[stale]

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

[domyen]

@atanasster what do you think of the expand/collapse pattern below? Note how the child rows are indented to reflect the parent-child relationship.

[atanasster]

Looks great @domyen and the indentation is very user friendly , just a reminder to possibly default expand the first interface.

[shilman]

@domyen @atanasster I like the design you propose, with:

• the component's props expanded by default
• the ancestor interface's props collapsed at the bottom by default

I think this can use the sections API of PropsTable:

export interface PropsTableSectionsProps {
  sections?: Record<string, PropDef[]>;
  expanded?: [string];
}

If expanded is provided, it will make the rows expandable/collapsible, with the provided sections expanded by default. The other props tables (Vue, Angular, WC, etc.) won't provide expanded, so their behavior shouldn't change.

[domyen]

Sounds good @shilman

[atanasster]

@shilman @domyen -
what about adding the number of props to the section header as in the initial screenshot?
Just a note that we cant really know which are own props - except try to substring it, or simply use the first interface listed ( i think its more visible in my jnitial screenshot)

• and the big question - 5.3 or later? ;)

[domyen]

what about adding the number of props to the section header as in the initial screenshot?

That makes sense to me! We do that in the animated gif in the column beside the name ("1 remaining unreviewed") – could do the same thing here.

[shilman]

@atanasster At this point it would be hard to fit into 5.3, but would be happy to merge it as soon as the release goes out!

[atanasster]

@shilman - I did some further tests:

adding very simple component

import React, { FC } from 'react';

interface ButtonProps {
  /** Own ButtonProps label */
  label?: string;
}

/** Component description imported from comments inside the component file
 * This React component has its own properties but also accepts all the html `button` attributes.
 */
export const TypescriptButton: FC<ButtonProps & JSX.IntrinsicElements['button']> = ({
  label,
  ...rest
}) => (
  <button type="button" {...rest}>
    {label}
  </button>
);

results in *260+ properties (quite disturbing), here is a screenshot:

Also an unfortunate finding is that react-docgen-typescript-loader does not actually support the parent name/fileName (strothj/react-docgen-typescript-loader#50) , so there is really no way to implement this feature with the typescript presets coming with docs:

So we would have to first change the loader to https://github.com/atanasster/webpack-react-docgen-typescript which does carry the parent property is we can differentiate where the property belongs:

[atanasster]

I hope this makes sense but in short without adding the & JSX.IntrinsicElements['button'], a typescript user wouldn't even be able to specify any of button attribute ie disabled etc.
On the other hand, such components will display hundreds of properties in the props table in storybook.

[patricklafrance]

Sorry to jump in like that.

Is there value in rendering the props from native JSX components?

Wouldn’t it be easier to exclude those props?

This is what I was planning to do when I get back from vacation.

[atanasster]

@patricklafrance - the more (fedback) the merrier :)
you can still filter properties out, although I am not sure how you would do it with react-docgen-typescript-loader since it does not distinguish any composed props.

Can you please elaborate what you had in mind already?

[atanasster]

@patricklafrance I looked at your use case.
semantic-ui-react are not exposing the intrinsic properties, although they do have an extensive list of duplicated properties that are inherited.

IMHO that's a (typescript, even though semantic-ui-react is not typescript and just exposes d.ts files) bug - and semantic-ui-react is taking a shortcut to avoid typescript errors, but basically ignoring type safety for all intrinsic properties that are not listed.

Here is a button example of what I encountered:

https://github.com/Semantic-Org/Semantic-UI-React/blob/2dbab6bec372f3013d4c18448c2600c9d531e378/src/elements/Button/Button.d.ts#L17

[patricklafrance]

@patricklafrance - the more (fedback) the merrier :)
you can still filter properties out, although I am not sure how you would do it with react-docgen-typescript-loader since it does not distinguish any composed props.

Can you please elaborate what you had in mind already?

What I have in mind is implemented in the storybook config of cra-ts-kitchen-sink

[atanasster]

What I have in mind is implemented in the storybook config of cra-ts-kitchen-sink

I am not sure I understand - the cra-ts-kitchen-sink Button component is not really representative:

const Button: FunctionComponent<Props> = ({ children, type = 'default', onClick }) => {
  return (
    <button type="button" onClick={onClick}>
      {type}: {children}
    </button>
  );
};

You can pass only two props to the Button - type and onClick - other properties like idor any aria- etc attributes will generate typescript errors.

[patricklafrance]

That might be a different use case then!

I haven’t use typescript much (and I am on my phone), I might not understand correctly the problem you are solving here.

For the Button component in cra-ts-kitchen-sink, Before the props filter had been configured on the typescript loader about 200 props from the React FC interface were rendered.

[atanasster]

@patricklafrance - you mean the code that says Currently not working :)

propFilter: prop => {
  // Currently not working, prop.parent is always null.
  if (prop.parent) {
    return !prop.parent.fileName.includes('node_modules/@types/react/');
 }
return true;

IMHO opinion we do need to support combined props - its not just JSX.IntrinsicElements but the users can compose interfaces in many ways. Most react UI libraries are just starting to really support typescript, so I guess its not that big of an issue yet, bit something that is good to be prepared for.

Lets talk more when you are back from vacation, and if you come around Tampa, give me a shout out :)

[atanasster]

Btw the filter above would work with my webpack-react-docgen-typescript loader, thats why I said filtering is an option. However not with the default react-docgen-typescript-loader filtering is not currently an option.

[patricklafrance]

@patricklafrance - you mean the code that says Currently not working :)

propFilter: prop => {
  // Currently not working, prop.parent is always null.
  if (prop.parent) {
    return !prop.parent.fileName.includes('node_modules/@types/react/');
 }
return true;

IMHO opinion we do need to support combined props - its not just JSX.IntrinsicElements but the users can compose interfaces in many ways. Most react UI libraries are just starting to really support typescript, so I guess its not that big of an issue yet, bit something that is good to be prepared for.

Lets talk more when you are back from vacation, and if you come around Tampa, give me a shout out :)

Haha it end up working after updating the library, forgot to remove the comment :)

Sounds good!

[atanasster]

strange, its still an open issue on webpack-react-docgen-typescript afaics (posted a link above)

Just tested on the latest cra-ts-kitchen-sink, and the cra preset doesn't seem to work to show any props at all :(

I don't see any component in cra-ts-kitchen-sink using JSX.IntrinsicElements that would generate 200 properties - when you are back from vacation would love to get our information merged :)

[atanasster]

aaah, I see - the filter by parent on react-docgen-typescript-loader should work, the issue was just about parent not being brought back.

[dgreene1]

How would I enable this feature?

[shilman]

We've just released zero-config typescript support in 6.0-beta. Please upgrade and test it!

• Release 6.0.0-beta.12+
• Migration instructions
• Updated docs

Thanks for your help and support getting this stable for release!