Skip to content

Instantly share code, notes, and snippets.

@mzabriskie
Last active December 1, 2016 15:56
Show Gist options
  • Save mzabriskie/2b3544f2d9d949bd0fe31f090cef301b to your computer and use it in GitHub Desktop.
Save mzabriskie/2b3544f2d9d949bd0fe31f090cef301b to your computer and use it in GitHub Desktop.

Portal Component Refactor

Proposal for how to refactor InstUI's use of <Portal />.

The Problem

There are several issues with how <Portal /> is currently being used.

  1. We have two versions of <Portal /> in our code base. A version that we wrote ourselves and a version packaged with react-overlays. This causes unecessary overhead.
  2. There are inconsistencies with the developer facing API for how to specify the trigger and the content for components that use a <Portal />. This makes it confusing as the usage of these components is not predictable.
  3. We have a few components that are effectively wrappers used to compose other components. This makes them less reusable as they are inherently more prescriptive. It also strays from our mission of providing an atomic component library.

The Proposal

Refactor components using <Portal /> in the following ways:

  1. Remove react-overlays from our repository in favor of our own implementation.
  2. Produce a predictable API for components that have trigger/content.
  3. Remove composite wrapper components in favor of atomic components.

The Components

There are roughly a half dozen components at play here. Some are lower level utility components that don't actually render anything themselves. The rest use these lower level components to provide stylized content.

  • <Portal />

    The <Portal /> is the root of all these components. It allows a developer to declare content inline with the rest of their template, but have it render out of context in another portion of the document.

    • Renders it's children directly.
    • Supports isOpen.
  • <Position />

    The <Position /> adds the ability to position it's content adjacent to some target.

    • Renders it's children via <Portal />.
  • <Popover />

    The <Popover /> inserts it's content adjacent to it's trigger styled as a context box.

    • Renders it's children as a <ContextBox /> via <Position />
  • <Overlay />

    The <Overlay /> inserts an opaque background behind it's content and adds transition and events for a11y.

    • Renders it's children via <Portal />.
    • Supports isOpen.
    • Stylizes it's content.
  • <Tray />

    The <Tray /> inserts it's content placed at the top, right, bottom, or left edge of the screen.

    • Renders it's children via <Portal />.
    • Supports isOpen.
    • Stylizes it's content.
  • <Modal />

    The <Modal /> inserts it's content styled as a dialog box.

    • Renders it's children via <Overlay />.
    • Supports isOpen.
    • Stylizes it's content.
  • <Tooltip />

    The <Tooltip /> inserts it's content styled as a tooltip.

    • Renders it's children via <Popover />
    • Supports isOpen
    • Stylizes it's content.

The Approach

Trigger and content APIs currently fall under one of these methods:

  • Trigger is external to component with content passed as children.

    This method works well for lower level components and provide a lot of control over how/when they are open and what trigger and content are used.

    Example:

    <div>
      <button onClick={() => {
        this.setState({
          isPortalOpen: !this.state.isPortalOpen
        })
      }}>
        Toggle Open
      </button>
      <Portal isOpen={this.state.isPortalOpen}>
        <div>Portal content here...</div>
      </Portal>
    </div>

    Components using this method: <Portal />, <Overlay />, <Modal />, <Tray />

  • Trigger and content are custom components.

    This method is well suited for components that need reference to their trigger.

    Example:

    <Popover>
      <PopoverTrigger><button>Click Me</button></PopoverTrigger>
      <PopoverContent><div>Popover content here...</div></PopoverContent>
    </Popover>

    Components using this method: <Position />, <Popover />

  • Trigger is a prop with content passed as children.

    This method feels awkward. It is currently only used by one component and it could easily be eliminated.

    Example:

    <PopoverMenu trigger={
      <button>&hellip;</button>
    }>
      <MenuItem>Foo</MenuItem>
      <MenuItem>Bar</MenuItem>
      <MenuItem>Baz</MenuItem>
    </PopoverMenu>

    Components using this method: <PopoverMenu />

  • Trigger is created by wrapping children with content being a prop.

    This method is only used in one place and it is counter-intuitive. The content would feel more natural as props.children, which is opposite of it's current implementation. There are a couple ways to refactor this.

    Example:

    <Tooltip tip="Tooltip content here...">
      Open Tooltip
    </Tooltip>

    Components using this method: <Tooltip />

After refactoring APIs will be consolidated:

  • Trigger is external to component with content passed as children.

    Remains unchanged.

  • Trigger and content are custom components.

    Introduce a generic <Trigger /> and <Content />.

    Example:

    <Popover>
      <Trigger><button>&hellip;</button></Trigger>
      <Content>
        <Menu>{/* ... */}</Menu>
      </Content>
    </Popover>
  • Trigger is a prop with content passed as children.

    Proposed:

    <Popover>
      <Trigger on="click"><button>&hellip;</button></Trigger>
      <Content>
        <Menu>
          <MenuItem>Foo</MenuItem>
          <MenuItem>Bar</MenuItem>
          <MenuItem>Baz</MenuItem>
        </Menu>
      </Content>
    </Popover>

    This would be a direct replacement of the current implementation by using a <ContextBox />.

    Alternatively:

    <Position>
      <Trigger><a>Open sesame</a></Trigger>
      <Content>
        <Menu>
          <MenuItem>Foo</MenuItem>
          <MenuItem>Bar</MenuItem>
          <MenuItem>Baz</MenuItem>
        </Menu>
      </Content>
    </Position>

    This would allow using a <Menu /> without the decoration of a <ContextBox />.

  • Trigger is created by wrapping children with content being a prop.

    Proposed:

    <Tooltip>
      <Trigger><input /></Trigger>
      <Content><Heading>Foo</Heading></Content>
    </Tooltip>

    Internally this would just be using <Popover /> but include needed aria attributes.

The Gameplan

  1. Create a TriggerContent helper which introduces <Trigger />, <Content />, and pick.
  2. Create <Position /> component that incorporates TriggerContent.
  3. Refactor <Popover /> to implement new <Position /> component.
  4. Refactor <Tooltip /> to implement refactored <Popover />.

Example:

/* Popover */
import React, { Component } from 'react'
import { Trigger, Content, pick } from '../../util/TriggerContent'
import ContextBox from '../ContextBox'

export default class Popover extends Component {
  render () {
    const trigger = pick(Trigger, this.props.children)
    const content = pick(Content, this.props.children)

    return (
      <div>
        {trigger}
        <Portal>
          <ContextBox>{content}</ContextBox>
        </Portal>
      </div>
    )
  }
}

export { Trigger, Content }

/* Elsewhere */
import React, { Component } from 'react'
import Popover, { Trigger, Content } from './components/Popover'

export default class Foo extends Component {
  render () {
    return (
      <Popover>
        <Trigger><button>Open Popover</button></Trigger>
        <Content>This is some content, Lorem Ipsum and stuff</Content>
      </Popover>
    )
  }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment