RFC: Standartize content prop

[layershifter]

I'm think that we need to remove content prop from all our components.
This will be breaking change.

Why?

Confusing

It's represents different types in our components:

• contentShorthand
• itemShorthand

Poor coverage

There are many components that doesn't have content prop. Sometime they have another shorthand instead of content.

Useless

It does same job as children prop:

<Component>Children</Component>
<Component children='Children' />
<Component content='Children' />

Poor tests

We doesn't have special tests for contentShorthand, our existing is not so powerfull as rendersChildren.

Simplify render function

Our render funtions will be more simple, while we have less usages of lodash (_.isNil).

- {_.isNil(children) ? content : children}
+ {children}

---

@levithomason @jcarbo I'm glad to hear your throughts :smile

[levithomason]

I agree on confusing, poor coverage, and simplifying the render, however, it is not only useful but required to facilitate the shorthand factories.

The purpose of the content prop is that it supports shorthand creation. This is different than children in two very important ways:

1. Children does not support props objects
2. Children does not create an element, but renders the value as-is

Example, a Popup takes content which ultimately creates a content div within the Popup:

<Popup content='Some content' />

<div class="ui popup transition visible">
  <div class="content">Some content</div>
</div>

Whereas using children would, of course, render the text directly as a child. We also cannot pass children to the factory in every case because there would be no way for the user to pass certain values, such as an array of strings or a props object.

There are some usages which are unnecessary, such as the ItemMeta's content. This usage is a direct replacement of the children prop and offers no new features. I think the only reason we should keep these props is for API consistency and this design principal:

All components should have a content prop and it should create the required primary content of the component from a number, string, props object, or another element.

I think then we must keep the content prop for shorthand purposes. Let me know your thoughts on this.

[layershifter]

There are some usages which are unnecessary, such as the ItemMeta's content. This usage is a direct replacement of the children prop and offers no new features.

I want to eliminate possible misunderstandings. My proposal is remove content: contentShorthand that makes same job as children (ItemMeta), while keep content: itemShorthand (Popup) that is part of our design.

[levithomason]

I can see that argument. Keeping consistent props across all components is nice, however, our shorthand props are 1:1 with subcomponents. So, if there is no Foo.Content subcomponent, then it is reasonable to expect there should be no <Foo content /> prop.

I was leaning toward removing the prop for this case, however, I have one other consideration. Shorthand props, such as icon are incompatible with children for longstanding reasons. If a component has shorthand props and the user wants to use them in conjunction with children, then there is no way to do so:

<Button icon='user'>Profile</Button>          // does not work

<Button icon='user' content='Profile' />      // works

Essentially, the content prop is shorthand for children if you are using shorthand props.

We could allow the use of children in place of the content prop in most of these cases. My only concern there is the cases where we cannot do this so easily. Cases where the content is used with logic to change the markup and/or used as a value to createShorthand. See Message which passes content to create a shorthand paragraph, this would not work with passing children. Also, Header's shorthand content which has a Content subcomponent but content is used to alter the markup and not for shorthand Content creation, we don't want to alter the markup every time there are children. What if the user wants exactly the children that they passed?

The inconsistency issue is also raised again, that as a developer I have no way of knowing if a component can use children or must use content with my shorthand props. This goes back to the longstanding reasons conclusion.

---

In conclusion, I'm thinking it is still good to keep content for all components for consistency and predictability:

• children are always rendered exactly as you passed them, never altered/wrapped
• shorthand props are never compatible with children
• every component has a content prop that is the primary content
• all components behave the same all the time

[layershifter]

Main problem that I see there is incompability 😕

<Popup content='Some content' />

<div class="ui popup transition visible">
  <div class="content">Some content</div>
</div>

<Popup content={{ children: 'Some content' }} />

<div class="ui popup transition visible">
  <div class="content">Some content</div>
</div>

<Button icon='user' content='Profile' />

<button class="ui button">
  <i class="user icon"></i> Profile
</button>

<Button icon='user' content={{ children: 'Profile' }} /> // does not work

[levithomason]

Oh boy, this is a great point. We simply cannot have some content props work with shorthand, and others not. Exploring ideas here, it is possible to create a component from this:

<Button content={{ children: 'Profile' }} />

// => <button class="ui button"><span>Profile</span></button>

This would mean that all usages of the content prop must be passed to createShorthand. We are bound to run into CSS issues since content='foo' would also create an element.

I'll have to think on this one. At this point, it seems the least destructive path is to keep the content prop but also allow inconsistent shorthand usage. I don't like this idea, let's think on it and look for something better.

[layershifter]

I have some proposals.

content and contentRenderer

We have renderer functions in some components. It's a quite good pattern, I think we can use it there:

function PopupContent(props) {
  // ...
 return <ElementType>{_.isNil(children) ? contentRender(content) : children}</ElementType>
}

PopupContent.defaultProps = {
  contentRenderer: defaultContentRenderer
}

PopupContent.propTypes = {
  // ...
  content: customPropTypes.contentShorthand,
  contentRenderer: Proptypes.func,
}

const defaultContentRenderer = content => {
  if(typeof content === 'string' || typeof content === 'number') return content
  return createShorthand('div', content)
}

So, if we have string or number on content we will simply return them. If we have object, let's use shorthand power, but we will wrap it into child element.

rename content prop

Yes, it will be breaking, but content on Popup and PopupContent still will make different things. It's a good step to clarify usage of shorthands.

[levithomason]

So, if we have string or number on content we will simply return them. If we have object, let's use shorthand power, but we will wrap it into child element.

I like this on first glance except for two hang ups:

1. Added complexity
2. There is no way to know which element to wrap it with. A div cannot be a child of may components. A span might be safer, but it still may break CSS in some cases.

I'm now thinking that we do not worry about this until we have reported issues about it. I personally have never run into it. Do you have any use cases or bugs you've run into that requires a refactor?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 30 days if no further activity occurs. Thank you for your contributions.