feat(ListItem): add shorthand

[levithomason]

The List update broke the Introduction page:

There may be other legacy usages as well. Suggest searching the codebase and updating all usages of List to the v1 API.

[jeffcarbs]

It looks like List/List.Item shorthand was removed.

<List>
  <List.Item icon='check mark'>jQuery Free</List.Item>
  <List.Item icon='check mark'>Declarative API</List.Item>
  <List.Item icon='check mark'>Augmentation</List.Item>
  <List.Item icon='check mark'>Shorthand Props</List.Item>
  <List.Item icon='check mark'>Sub Components</List.Item>
  <List.Item icon='check mark'>Auto Controlled State</List.Item>
</List>

I'd maybe update this issue to add List shorthand.

[levithomason]

Agreed and done, thanks.

[jeffcarbs]

I had a chance to look at the all the possibilities for lists (there are a lot!) and put together some thoughts on this. I'd say this is roughly what we should support via shorthand:

// With icon or image
<List.Item>
  {List.Icon || Image}
  <List.Content>
    <List.Header />
    <List.Description />
    {content}
  </List.Content>
</List.Item>

// Without icon or image
<List.Item>
  <List.Header />
  <List.Description />
  {content}
</List.Item>

Going to post what I found to be the unique variations with the shorthand below each:

// Super basic
<List.Item>1</List.Item>

// Shorthand
<List.Item content='1' />


// Icon + Content (content has header + description)
<List.Item>
  <List.Icon name='map marker' />
  <List.Content>
    <List.Header as='a'>Krowlewskie Jadlo</List.Header>
    <List.Description>An excellent polish restaurant, quick delivery and hearty, filling meals.</List.Description>
  </List.Content>
</List.Item>

// Shorthand
<List.Item
  icon='map marker'
  header={{ as: 'a', content: 'Krowlewskie Jadlo'}}
  description='An excellent...'
/>


// Image + Content (content has header + description)
<List.Item>
  <Image avatar src='http://semantic-ui.com/images/avatar2/small/rachel.png' />
  <List.Content>
    <List.Header as='a'>Rachel</List.Header>
    <List.Description>Last seen watching <a><b>Arrested Development</b></a> just now.</List.Description>
  </List.Content>
</List.Item>

// Shorthand
<List.Item
  image={{ avatar: true, src: 'http...' }}
  header={{ as: 'a', content: 'Rachel'}}
  description='Last seen watching...'
/>


// Icon + Content (content just has text)
<List.Item>
  <List.Icon name='users' />
  <List.Content>Semantic UI</List.Content>
</List.Item>

// Shorthand
<List.Item
  icon='users'
  content='Semantic UI'
/>


// Header + naked content, not wrapped in List.Content
<List.Item>
  <List.Header>New York City</List.Header>
  A lovely city
</List.Item>

// Shorthand
<List.Item
  header='New York City'
  content='A lovely city'
/>


// Header + description, not wrapped in List.Content
<List.Item>
  <List.Header as='a'>Header</List.Header>
  <List.Description>
    Click a link in our <a>description</a>.
  </List.Description>
</List.Item>

// Shorthand
<List.Item
  header={{ as: 'a', content: 'Header'}}
  description='Click a link...'
/>

// Multiple "content"s mixed with other data
<List.Item>
  <List.Content floated='right'>
    <Button>Add</Button>
  </List.Content>
  <Image avatar src='http://semantic-ui.com/images/avatar2/small/lena.png' />
  <List.Content>
    Lena
  </List.Content>
</List.Item>

// Shorthand
// Note: I don't think we can handle multiple contents.


// Nested list
<List.Item>
  <div>Benefits</div>

  <List.List>
    <List.Item href='#'>Link to somewhere</List.Item>
    <List.Item>Rebates</List.Item>
    <List.Item>Discounts</List.Item>
  </List.List>
</List.Item>
<List.Item>Warranty</List.Item>

// Shorthand
<List.Item
  content={<List /** Shorthand or full JSX for list */ />}
/>

// Content with props
<List.Item>
  <Image avatar src='http://semantic-ui.com/images/wireframe/square-image.png' />
  <List.Content verticalAlign='middle'>
    Middle
  </List.Content>
</List.Item>

// Shorthand
<List.Item
  image={{ avatar: true, src: '...' }}
  content='Middle'
  verticalAlign /** NOTE: This is passed-through to List.Content */
/>

Note that last example where a prop is passed through to List.Content. Our normal pattern/naming are a little conflicting here:

• content could be a shorthand prop for List.Content
• content could be the contentShorthand that takes the place of children

I think going with the latter makes sense, even though it forces us to pass-through props, because the List.Content may or may not be necessary. This allows a user to just pass header and description as props to the List.Item without needing nested hashes (e.g. content={{header, description}}).

[levithomason]

// Shorthand
// Note: I don't think we can handle multiple contents.

Agreed!

Corrections

I updated my comment with your corrections and removed them here to keep this comment more readable - @jcarbo

Suggested Updates

// Content with props
<List.Item>
  <Image avatar src='http://semantic-ui.com/images/wireframe/square-image.png' />
  <List.Content verticalAlign='middle'>
    Middle
  </List.Content>
</List.Item>

// Shorthand
<List.Item
  image={{ avatar: true, src: '...' }}
- content='Middle'
- verticalAlign /** NOTE: This is passed-through to List.Content */
+ content={{ verticalAlign: true, content: 'Middle' }}
/>

I know content/content looks a little strange at first, though, each shorthand prop is mapped to a sub component part and the content shorthand prop is always the primary content when there are no sub components left. In this case, it is the primary content of the List.Content component part. We have this also in the FeedContent:

<Feed.Content>Hi<Feed.Content>

<Feed.Content content='Hi' />

<Feed content='Hi' />

<Feed.Content foo='bar'>Hi<Feed.Content>

<Feed.Content foo='bar' content='Hi' />

<Feed content={{ foo:'bar', content: 'Hi' }} />

I would also note that it is likely a less likely use case to have content={{ content: '', verticalAlign: true }}. Most the time, there won't be two content terms at play. When there is, odds are the user will be familiar with the conventions and it shouldn't be too much of a curve ball.

Thoughts?

[jeffcarbs]

The double content={{ content: '' }} doesn't bother me as much. It's that the ItemContent isn't always supposed to be there so we can't just call ListContent.create in all contexts. Always having the wrapping content actually causes CSS issues. Therefore, here's what would change from the existing PR I opened #637:

  const iconNode = ListIcon.create(icon)
  const imageNode = Image.create(image)

+ if (_.isPlainObject(content)) {
+   return (
+     <ElementType {...rest} className={classes} {...valueProp}>
+       {iconNode || imageNode}
+       {ListContent.create(content, { header: header, description: description })}
+     </ElementType>
+   )
+ }

  const headerNode = ListHeader.create(header)
  const descriptionNode = ListDescription.create(description)

  if (iconNode || imageNode) {
    return (
      <ElementType {...rest} className={classes} {...valueProp}>
        {iconNode || imageNode}
        {(content || headerNode || descriptionNode) && (
          <ListContent>
            {headerNode}
            {descriptionNode}
            {content}
          </ListContent>
        )}
      </ElementType>
    )
  }

  return (
    <ElementType {...rest} className={classes} {...valueProp}>
      {headerNode}
      {descriptionNode}
      {content}
    </ElementType>
  )
}

Doesn't add that much complexity I guess. Thoughts?

[levithomason]

Looks good to me, I think the public API wins out over a little funny business in the code. Suggest a code comment to clarify why the plain object check.

[jeffcarbs]

The thing I don't love about that approach is that it deviates from our current shorthand pattern. In this case, content if content is:

• props, it's treated as props to Item.Content (and thus the wrapper around header/description) (makes sense)
• element
  • if icon/image present, it's treated as the content of Item.Content (and a sibling to header/description)
  • if not icon/image present, it's treated as sibling to header/description

This might be a little more consistent:

  const iconNode = ListIcon.create(icon)
  const imageNode = Image.create(image)

  const headerNode = ListHeader.create(header)
  const descriptionNode = ListDescription.create(description)

  if (iconNode || imageNode || _.isPlainObject(content)) {
    return (
      <ElementType {...rest} className={classes} {...valueProp}>
        {iconNode || imageNode}
-       {(content || headerNode || descriptionNode) && (
-         <ListContent>
-           {headerNode}
-           {descriptionNode}
-           {content}
-         </ListContent>
-       )}
+       {(content || headerNode || descriptionNode) &&
+         ListContent.create(content || '', { header: header, description: description })}
      </ElementType>
    )
  }

  return (
    <ElementType {...rest} className={classes} {...valueProp}>
      {headerNode}
      {descriptionNode}
      {content}
    </ElementType>
  )

Note that it's valid to only pass a header or description so we need a non-nil value for content so that the content gets generated.

But even in that case, if:

• props, it's treated as props to Item.Content (and thus the wrapper around header/description) (makes sense)
• element
  • if icon/image present, it's treated as a replacement of Item.Content (and overrides header/description which is fine)
  • if not icon/image present, it's treated as sibling to header/description

[jeffcarbs]

After thinking about it I'd kinda prefer the first one and just comment on the inconsistency/break from pattern.

• If you pass content as an element/literal, it's actually treated as the sibling node to header/description (whether wrapped in Item.Content or not).
• If you pass content as props, it forces the presence of Item.Content and passes those props to it. If you pass a content prop within that props object, it will be treated as the sibling node to header/description.