RFC: Standardize shorthand prop names for primary content

[levithomason]

Issue

Many components provide a shorthand prop for defining the primary content of the component. We currently have inconsistent prop names for this shorthand prop. It makes it confusing as a developer to have to use text in some places, content in others, and more.

Additionally, the text prop (and others) are already used in some components to control features. We need a standard that is consistent and avoids collisions.

Proposal

Here are the proposed rules for choosing the primary content shorthand prop name:

Naming

1. Use the component part name

If there is a subcomponent which holds the primary content of the component, then the subcomponent name should be used for the shorthand prop name:

<Step description='...' />
<Step.Description>...</Step.Description>

This makes intuitive sense as you are passing down a prop from the parent to the subcomponent with the same name. It also follows what we are doing with the icon and image props, where you can configure the children with the same prop name.

It allows us to also add standardized feature support for various component parts via our factories, createDescription(...), createMeta(...). Contrived examples, we could standardize props like childKey, elementType, or rending links, etc.

2. Use content

If the component does not have a subcomponent name to use, then use content:

<Step.Description content='...' />   // is a subcomponent (no further subcomponents)
<Label content='...' />              // has no subcomponent

This makes sense in the context of SUI as "content" is a standard component part that means "the primary content of the component". If there are no child components left to configure, then you are adding the "content" of the component.

Again, this would allow us to createContent(content) and add standardized support via our factories if we wanted.

A good counter argument to content on a component that has no subcomponents is, "Why not just use children?" This is already built into React of course.

<Step.Description children='...' />
<Label children='...' />

PropTypes

PropType validation has also come up in https://github.com/TechnologyAdvice/stardust/pull/390#discussion_r74690941. Initially we thought to limit these to strings only. This allows the props to be used as child keys in arrays. It also simplifies things for the user.

However, some components like <Statistic.Value /> may very well accept a number on <Statistic value={100} />. It would be troublesome to force users to convert numbers to strings in this case. Also, numbers do not present the same issues for child keys as accepting objects.

customPropTypes.shorthand

I propose we create this propType to consistently validate shorthand props. Not only would it standardize shorthand props, we could then bake in disallow(['children']) as well. This would pair very nicely with standardize factories for shorthand.

CONTRIBUTING

Any PR resolving this issue should update the CONTRIBUTING.md with the final shorthand props spec.

[layershifter]

Looks good 👍

Also, might be added commonTests for 1 and 2 situations.

[layershifter]

A good counter argument to content on a component that has no subcomponents is, "Why not just use children?" This is already built into React of course.

I can agree there, seems content prop creates unnecessary overhead.

[levithomason]

While we're at this, note the Header allows shorthand and children to be defined at the same time. Any PR fixing this issue should also fix that. They should be exclusive.

The Header needed a complete refactor due to component augmentation, I've baked this in as well. See, #414.

[levithomason]

This work is going on in #452. The content prop has proved useful after all. Since, you cannot define shorthand props and children. This is not allowed:

<Label icon='settings'>Settings</Label>
<Label icon='settings' children='Settings' />

Where as this is:

<Label icon='settings' content='Settings' />

We could have made the children prop type conditionally required to be a string given the presence of any of the shorthand props. However, this is very convoluted. It is much more clear to say content is the primary content shorthand prop for any component and it is limited to the shorthand type.

IMO, there is also just something not quite right about passing children as a prop when using JSX.

[levithomason]

All components have been updated.