<Popup /> Component

[levithomason]

http://semantic-ui.com/modules/popup.html

[layershifter]

1. Use react-portal

As I understand it's already used for modal, so it might be used there.

2. Popup syntax

SUI performs following syntax for popup.

<div class="ui icon button" data-content="Add users to your feed">
  <i class="add icon"></i>
</div>

As I understand it's not applicable to React :) I'm thinking about following:

const button = <Button icon><Icon name='add' /></Button>

<Popup target={button}>Add users to your feed</Popup>
<Popup targets={[button]}>Add users to your feed</Popup>

Looks pretty, when there is only one popup :) On multiple it will look illy.

I know @levithomason has better idea 😺

[levithomason]

Hm, I'd have to play with the code for a bit, but here are a few more ideas to consider:

1. Children.only()

My first thought was to use Children.only() to require a single child. This child is the rendered element, and the Popup wrapping it takes it's values via props:

const OnlyChildExample = () => (
  <Popup title='Elliot Fu' content='Elliot has been a member since July 2012'>
    <Image src='/images/avatar/small/elliot.jpg' avatar />
  </Popup>
)

This is clean, but makes complicated popup markup painful as you have to pass it through props only.

2. Render Sub Component (hideous)

Similar to the above, except it makes complicated Popup content easier as you can write it in JSX. In this case the only child of the Render sub component is used as the element to render:

const RenderSubComponents = () => (
  <Popup>
    <Popup.Title>Elliot Fu</Popup.Title>
    <Popup.Content>Elliot has been a member since July 2012</Popup.Content>
    <Popup.Render>
      <Image src='/images/avatar/small/elliot.jpg' avatar />
    </Popup.Render>
  </Popup>
)

const RenderProps = () => (
  <Popup title='Elliot Fu' content='Elliot has been a member since July 2012'>
    <Popup.Render>
      <Image src='/images/avatar/small/elliot.jpg' avatar />
    </Popup.Render>
  </Popup>
)

In the RenderProps example, you could allow an only child without the extra Popup.Render wrapping it. So it could accept either sub components Title/Content/Render, or shorthand props and a single child to render. In that case, the RenderProps example would be identical to the OnlyChild example.

3. Augmentation

Depends on #414. The Popup could render as some other component. In this example, the Popup has only two props/sub components title and content. This makes its API resemble most of our other components.

const AugmentationSubComponents = () => (
  <Popup as={Image} src='/images/avatar/small/stevie.jpg' avatar>
    <Popup.Title>Stevie Feliciano</Popup.Title>
    <Popup.Content>Stevie has been a member since August 2013</Popup.Content>
  </Popup>
)

const AugmentationProps = () => (
  <Popup
    title='Stevie Feliciano'
    content='Stevie has been a member since August 2013'
    as={Image}
    src='/images/avatar/small/stevie.jpg'
    avatar
  />
)

---

I think one lesson I've taken from the Modal is that we should separate the actual element from the portal use. I think the Portal should just use the element (Modal, Popup, etc) rather than have both baked into one. I'm not entirely sure what the API would look like here, but you can reference React Bootstrap's overlay trigger example as an idea.

The issue we have with the Modal implementation is that it is harder to test as it does not render a Modal, it renders a Portal. More so, the Portal has no Modal child as it renders that to the body. This means all tests must mount the Modal and test the actual DOM. I'd much rather have had the Modal be a stateless component, and something like React Bootstrap's <OverlayTrigger /> (see previous link) handle the portal logic.

I think we should also consider the Dimmer API as it is related to this type of behavior. We should strive to make the Modal, Dimmer, and Popup patterns similar or identical if we can. All of these components involve a Portal and/or rendering children wrapped in another component.

[levithomason]

What I like about the target idea more than the above is that it separates the Popup from the component it is popping up over. This makes them more loosely coupled which I think is good.

Also, in keeping with our rule of "props or children but never both", I think the Popup.Render props example would look like this actually:

const RenderProps = () => (
  <Popup 
    title='Elliot Fu'
    content='Elliot has been a member since July 2012'
    render={<Image src='/images/avatar/small/elliot.jpg' avatar />}
  />
)

[brsanthu]

@levithomason (Not meaning to be pushy but just asking) Any idea when this might arrive?

[levithomason]

Here are my personal priorities. My first focus is #269, updating legacy components to our v1 API. Once all these are done, I'll focus on #243, migrating to Semantic-Org. I will continue to give time daily to bug fixes and PR reviews as they arise.

After those two issues are closed, I'll continue focusing on daily bugs and PRs. I also plan to then focus on new components. Components we need at @TechnologyAdvice will come first. Second, I'll focus on components I am personally interested in.

So, it will be a while before I personally get to this component. However, I would support a PR immediately.

[typehorror]

Hey @levithomason , I'm pretty excited to use stardust for my own things and would like to contribute to it! I would like to start by taking a stab at the popup component since it seems like it's not one of your priorities.

As you suggested this option, I would like to use a target as the origin of the popover to keep it detached from it. I added some example using what I have in mind. I want to make sure I follow the patterns for V1 and I would like to have your opinion before I start anything. Let me know.

A Popup without children

<SomeElement ref="my_element" />
<Popup target="my_element" tooltip="foo bar" inline />

a Popup with a content as child with a position

<SomeElement ref="my_element" />
<Popup target="my_element" position="top-center">
  Phasellus enim lacus, finibus nec pretium ut, 
  interdum sit amet sapien.
</Popup>

a Popup with a Header and a Content using a size variation

<SomeElement ref="my_element" />
<Popup target="my_element" variation="very wide">
  <Popup.Header>Foo Bar</Popup.Header>
  <Popup.Content>
    Phasellus enim lacus, finibus nec pretium ut, 
    interdum sit amet sapien.
  </Popup.Content>
</Popup>

Props would be something in those lines (inspired by the SUI doc) :

• target: node (required)
• inline: bool
• position: oneOf(['top-left', 'top-center', 'top-right',...])
• tooltip: string
• inverted: bool
• variation: arrayOf(['mini', 'tiny', 'small', 'large', 'huge', 'wide', 'very wide',...])

[levithomason]

@Debrice thanks for picking this issue back up! Let's avoid refs if at all possible, see #405.

We can still separate the Popup from the trigger component by passing the actual trigger component to the Popup. The Popup will render it's trigger in it's place. The Popup markup itself will be rendered to the body using Portal.

I imagine this would working something like the following:

const trigger = <Button>Has Popup</Button>

<Popup 
  trigger={trigger}
  title='Elliot Fu'
  content='Elliot has been a member since 2012'
/>

This would render a Button (the trigger) in place of the Popup, but on hover of the trigger the Popup would be rendered to the body by the Portal and absolutely positioned onto the Button.

As for props, let's try to keep parity with SUI's data- names and Popup settings. Hence, title and content to accept what they are using for data-title and data-content.

I'd gladly review and help if you wanted to start a PR.

[typehorror]

Sounds good, I'll start a PR and let you know when it's worth reviewing!

[typehorror]

@levithomason I'm facing an issue on render. Since the Popup is responsible for rendering trigger, in order for the Popup to render itself too (through Portal) I would have to wrap them in a parent DOM element (since I now have 2 elements to render).

This causes an issue since the style of the trigger would be corrupted by the container element:

render() {
  return <div>{this.props.trigger}{this.state.popup ? <Portal>popup</Portal>: null }</div>
}

I know I can do it with a ref (since the element would be detached), but after reading #405, I'm trying to stay away from these. I have to admit, I'm stuck on that one.

I put my work on a pull request #570 for you to check it out.

[levithomason]

Thanks, we'll continue the convo there.

[kapiljaisinghani]

Hi , how can i render semantic ui popup with existing DOM element as a trigger ? All the examples render the trigger component inline

import { Button, Popup } from 'semantic-ui-react'

const PopupExample = () => (
  <Popup
    trigger={<Button icon='add' />}
    content='Add users to your feed'
  />
)

export default PopupExample```

[layershifter]

Use SO and Gitter for questions, do not necropost please.