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 children as default prop for rt-template #120

Open
choffmeister opened this issue Apr 22, 2016 · 6 comments
Open

Use children as default prop for rt-template #120

choffmeister opened this issue Apr 22, 2016 · 6 comments

Comments

@choffmeister
Copy link

Currently this is working

class MyComp extends React.Componet {
  const items = ['a', 'b'];
  render() {
    return this.props.renderItems(items);
  }
}
<MyComp>
  <rt-template prop="renderItems" arguments="items">
    <div>
      <div rt-repeat="item in items">
        <!-- ... -->
      </div>
    </div>
  </rt-template>
</MyComp>

whereas prop="..." is mandatory. Wouldn't it make sense, to allow omitting prop and, when omitted, just to assume prop="children" as default? This would match with the common React pattern, to pass a function as children that generates the inner content to render dynamically. Or even completely remove the prop="..." and always use children for that (since we cannot have rt-template and additional children at the same time anyway).

This would then look like this:

class MyComp extends React.Componet {
  const items = ['a', 'b'];
  render() {
    return this.props.children(items);
  }
}
<MyComp>
  <rt-template arguments="items">
    <div>
      <div rt-repeat="item in items">
        <!-- ... -->
      </div>
    </div>
  </rt-template>
</MyComp>
@nippur72
Copy link
Contributor

It seems logical, anyway I don't understand when you say

since we cannot have rt-template and additional children at the same time anyway

@choffmeister
Copy link
Author

I mean, then i can either define children

<MyComp>
  <div>
  </div>
</MyComp>

or a rt-template

<MyComp>
  <rt-template>
  </rt-template>
</MyComp>

but not both at the same time.

@nippur72
Copy link
Contributor

can you provide an actual example? I've just tried it in the playground and it seems to work.

@nippur72
Copy link
Contributor

This is working for me:

myart.rt:

<div>   
   title = {this.props.title()}   
   body = {this.props.children}
</div>

myapp.rt:

<MyArt>
   <rt-template prop="title"><div>this goes into title</div></rt-template>
   <div>this goes into children</div>
</MyArt>

@choffmeister
Copy link
Author

choffmeister commented May 30, 2016

It is? Didn't know that. Then cross the line you quoted from me out. But still, making prop=children the default would be a nice thing :)

@nippur72
Copy link
Contributor

the only issue I see in making it default is that it would require {this.props.children()} instead of {this.props.children} (with ()).

This because rt-templates are actually functions, while in this scenario one would expect them to be rendered as vars.

Note that this issue makes rt-templates not compatible with other libraries (e.g. "material-ui") where usually an instance of React.Component is expected, not a factory function.

I am going to address it with a different syntax, see #104

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

No branches or pull requests

2 participants