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

Components enhancements #553

Open
stalniy opened this issue Jan 29, 2016 · 8 comments
Open

Components enhancements #553

stalniy opened this issue Jan 29, 2016 · 8 comments

Comments

@stalniy
Copy link
Contributor

stalniy commented Jan 29, 2016

Currently components lacks really powerful features:

  1. Lifecycle. There is only 2 methods initialize and template and last one is called first then first one. That means that developer can decide which template to render based on input information from the attributes. For example, I'd like to have a component which is something similar to input tags with different types but more complex, e.g. datepicker, autocomplete, etc. So, that I do this in HTML:
<form-field type="datepicker" value="birthDate"></form-field>

And my component is this:

rivets.component['form-field'] = {
  initialize: function(el, attrs) {
    return new FormField(attrs);
  },

  template: function(scope) {
    return TEMPLATES.get(scope.type);
  }
};

Currently it's not possible. The only possible solution is to create separate component for each form-field. But logically it should be one component with extendable points and some shared behaviour
2. initialize accepts 2 arguments: object of attributes and HtmlElement and this allows devs to write worse code as it mixes concepts (view - element, parsed attrs - kind of model). Another reason why it should be split into 2 because view (rivets binders) assigns view related variables (i.e. rv-each-*) right into my component controller what can lead to names collisions and unpredictable behaviour. So, something like { [componentName]: component.initialize() } should be view scope and not just component.initialize() result.
3. Sometimes you have a component with its default template but then you want to use it in another place with a bit different template. Currently there is no way to change components parts. This is what transclusion cares about. So, it allows to define parts inside components which can be overridden if necessary. So, you can do like this:

<form-field type="number">
  <label>My custom label</label>
  <div rv-part="error">My custom error message</div>
</form-field>

And Components template:

<div class="form-group">
   <label class="control-label" rv-transclude="label">{ label }</label>
   <input type="number" rv-value="value" class="form-control">
   <div rv-transclude="error" class="help-block error">{ error }</div>
</div>

In this example form-field will replace part which is called label inside default template with the provided tag.
4. Components doesn't not allow to return document fragment from their template. In the same time document fragment is the most performant way to put a bunch of HTML in DOM tree.
5. Sometimes you want to specify component's template as a string (when there is no need in function, you just have prepared HTML or want to put it right there without writing extra function)

rivets.components.tabset = {
  template: `
    <ul class="tabs">
      <li rv-each-tab="tabs" rv-click="selectTab">{ tab.title }</li>
    </ul>
    <div rv-transclude></div>
  `,

  initialize: ....
};

Here no need in function, so more convenient to pass just a string.
6. No support for dependent components like tabset and tab. For example currently this is not possible as components just remove inner html

<tabset>
  <tab title="Overview">Tab Content</tab>
  <tab title="Settings">Settings</tab>
</tabset>

Also it's not possible to access parent component inside child one at the moment.

@blikblum, @Leeds-eBooks, @jccazeaux want to hear your thoughts about this as I have already done all this (and a bit more in my fork). So, if we are ok with that we could split these tasks to smaller one and I will create a separate pull-request for each.

@blikblum
Copy link
Contributor

Component implementation now really falls short.
Although i would go for a subset of webcomponent specification something like Vue did.

@stalniy
Copy link
Contributor Author

stalniy commented Feb 10, 2016

One more point: currently there is no way to clean up component (for example clearInterval, etc). So, need to add one more method unbind which is called when component is unbound

@stalniy
Copy link
Contributor Author

stalniy commented Feb 11, 2016

Just to summarize efforts

  • Optional "template", "template" as string, element/document fragment based templates feat(components): make "template" method being optional. #545
  • change the order of "template" and "initialize" invokations (initialize first then template)
  • dependent components
  • transclusions or template parts substitution
  • formatters in component's attributes
  • unbind component's method for cleaning stuff when view of the component is unbound (like clearInterval, clear custom subscriptions, etc) feat(components): add support for unbind method in components #584
  • add controller method as replacement for initialize which accepts only passed attributes and dependencies. Basically when this method is specified we can use standard initialize-r. That solves 2 statement in issue description.
  • 2 way binding between passed attributes and component's controller

It's really easy to add controller support

  if (typeof this.component.controller === 'function') {
    this.component.initialize = defaultInitializer;
  }

  function defaultInitializer(el, attrs) {
    var scope = {};
    scope[this.type] = this.component.controller.call(this, attrs);

    return scope;
  }

@jccazeaux
Copy link
Contributor

I have a question about transclusion:

<form-field type="number">
  <label>My custom label</label>
  <div rv-part="error">My custom error message</div>
</form-field>

What will be rv-part? It looks like a binder but its value is more like an alias than an expression.

@stalniy
Copy link
Contributor Author

stalniy commented Feb 13, 2016

@jccazeaux in my fork I use block-name attr and its name may be configured. So, no worries about that

@jccazeaux
Copy link
Contributor

@stalniy I took some time to go deeper in components. I want to have your opinion on something I found.
Doc says

All attributes on the element will get evaluated as keypaths before being passed into the component's initialize function.
<todo-item item="myItem"></todo-item>
These keypaths will also be observed in both directions so that the component will update if the value changes from the outside and it will set the value if the component changes it from the inside.

It is not true, initialize function receives copy and not observed keypath. So if parent's value changes, component will not be notified. See this fiddle

In the code I found that Rivets binds component's attributes at root of component's model. This means we can access attributes directly without initialize function like that

<todo-item item="myItem">
  <!-- this is component's template -->
  { item }
</todo-item>

There, databinding is activated like doc describes it.

See this fiddle

There is only a glitch here : component's model is not initialized at first. You have to change parent's value at least once. This glitch is very easy to fix if necessary

Were you aware of this?

CC: @Duder-onomy @Leeds-eBooks @blikblum

@benadamstyles
Copy link
Collaborator

Hey @jccazeaux I have finally got round to this. I thought that this bug you found was because you were declaring the component as a function rather than as a class, so I forked your fiddle here and as I thought, when declaring the component as an es6 class it works correctly, updating the parent property. I don't know enough about JS classes to explain why this is the case but if you follow the docs, declaring a component using new, components as they are currently work correctly.

@jccazeaux
Copy link
Contributor

Hi @Leeds-eBooks your fiddle is interesting. It's not the using of JS class that changed the thing. Look at this.
What i saw since :

  • If initialize returns empty object, databinding still works but the component's model is not initialized.
  • If initialize returns undefined, databinding does not work (i think it's normal, it's just to say)
    See all of it here

So to set initial value in binding you have to do it in initialize. I don't find that very convenient.

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

4 participants