Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use React Fragment instead of as prop #3030

Closed
alzalabany opened this issue Jul 23, 2018 · 2 comments
Closed

use React Fragment instead of as prop #3030

alzalabany opened this issue Jul 23, 2018 · 2 comments
Labels

Comments

@alzalabany
Copy link

alzalabany commented Jul 23, 2018

Feature Request

react now support a container that doesnot render a div for itself. aka 'React.Fragment or <> for babel7'

Problem description

when we use "props.as" to select type of child to render, example in something like

<Transition.Group as={List} duration={200} divided size='huge' verticalAlign='middle'>
          {items.map(item => (
            <List.Item key={item}>
              <Image avatar src={`/images/avatar/small/${item}.jpg`} />
              <List.Content header={_.startCase(item)} />
            </List.Item>
          ))}
</Transition.Group>
  • its very hard to tell now what props are set for Transition.Group and which are intended for List component that i passed in props.as.
  • name colliissino can happen easily especially when i pass cutom components to 'as' example if i pass a component that needs a size prop.

Proposed solution

can be used without as prop, and we just update render function removing uneeded code.

// https://github.com/Semantic-Org/Semantic-UI-React/blob/master/src/modules/Transition/TransitionGroup.js
render() {
    debug('render')
    debug('props', this.props)
    debug('state', this.state)

    const { children } = this.state
    //- const ElementType = getElementType(TransitionGroup, this.props)
    //- const rest = getUnhandledProps(TransitionGroup, this.props)

    //- return <ElementType {...rest}>{_.values(children)}</ElementType>
    return <React.Fragment> {_.values(children)} </React.Fragment>
 }

now

  • i don't need to polute and do un-needed calcs.
  • this is much cleaner cause i dont have to polute my TransitionGroup in this example with props intended for child component
@welcome
Copy link

welcome bot commented Jul 23, 2018

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you've completed all the fields in the issue template so we can best help.

We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

@layershifter
Copy link
Member

Fragments are supported only from 16.2 while currently we require 16.0. The next breaking changes will require 16.3, #2844.

We will don't remove as prop there, for me it's a cool solution and it's a part our API. I've made a PR that sets as defaults to Fragment, see #3032.

Now, you can use Fragments, too:

<Transition.Group as={React.Fragment} />

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants