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

<m-insert> inside m-for #171

Closed
rizrmd opened this issue Oct 22, 2017 · 13 comments
Closed

<m-insert> inside m-for #171

rizrmd opened this issue Oct 22, 2017 · 13 comments

Comments

@rizrmd
Copy link

rizrmd commented Oct 22, 2017

I just tried this:

 Moon.extend("ui:layout.row", {
            data: function() {
                return {
                    d: [1,2,3,4]
                };
            },
            template: '<div><span m-for="a in d"><m-insert /></span></div>',
        })

and then:

capture

@rizrmd
Copy link
Author

rizrmd commented Oct 22, 2017

I think i know the cause:

in https://github.com/kbrsh/moon/blob/master/src/util/dom.js#L36

 // Append all children
 for(let i = 0; i < children.length; i++) {
      appendChild(children[i], node);
    }

should be:

    // Append all children
    for(let i = 0; i < children.length; i++) {
      if (typeof children[i] === 'object') {
        appendChild(children[i], node);
      }
    }

because insert property of each instance is shaped like these :

insert: {
    0: 0,
    1: 1,
    2: { .... component declaration ... },
    ....
}

so when appendChild(children[i], node) executes, it processes 0 and 1 which is not a children

@kbrsh
Copy link
Owner

kbrsh commented Oct 22, 2017

Huh, thanks for letting me know! The issue seems to be with m.flatten actually, as it is inserting numbers.

Other than that, multiple inserts aren't supported, and I'm not planning on adding them. It's because each insert refers to the same set of elements, and Moon can only bind them to one DOM node. As a result, it will break when diffing.

@rizrmd
Copy link
Author

rizrmd commented Oct 22, 2017

What do you mean by multiple inserts, is it placing multiple <m-insert> in a single component ?

if so, do you have any suggestion on how to achieve multiple inserts without relying on Moon ?

My intention is to build a List component, so the user can put each item template directly in their component. Something like:

<list name="list1" data="{{ [1,2,3,] }}">
     <-- declare list-item here -->
      <b>{{ list1.item }} </b>
</list>

I am thinking of cloning instance.insert, convert it into html, modify user's component template, and then create a new component with the new template ... is it a good idea ?

Any suggestion ?

@kbrsh
Copy link
Owner

kbrsh commented Oct 22, 2017

Yup, you cannot have multiple <m-insert/>'s in a single component. You can't really achieve this without Moon, and there really isn't a use case for it.

How do you think that list component will work? list1 will be bound to the parent, and the m-for will have the exact same element repeated even if multiple inserts were allowed.

Modifying the component template won't work either. Moon templates are compiled—for example:

<div class="component">
  <m-insert/>
</div>

is compiled into:

var compiledRender = function(m) {
  var instance = this;
  var staticNodes = instance.compiledRender.staticNodes;
  if(staticNodes === undefined) {
    staticNodes = instance.compiledRender.staticNodes = [];
  }
  return m("div", {attrs: {"class": "component"}}, {}, m.flatten([instance.insert]));
}

@rizrmd
Copy link
Author

rizrmd commented Oct 22, 2017

Let say we have this code for displaying a list:

<div m-for="item in [1,2,3,4]">
    <b>{{ item }}</b> <!-- list item detail -->
</div>

The requirement of my component is like this:

<list data="{{ [1,2,3,4] }}">
    <b>{{ item }}</b> <!-- list item detail -->
</list>

I know the list1 name is rather confusing, so i just omit it. And I aware that {{ item }} is magically present, I think it can be documented later — please bear with me.

Now.... What if we can pre-process that template just before it being compiled by Moon.
Maybe we can turn html above into something like these, just before it being compiled:

<list>
     <div m-for="item in [1,2,3,4]">
          <b>{{ item }}</b> <!-- list item detail -->
     </div>
</list>

User doesn't need to know <list> implementation detail, and Moon will get proper html template. Win-Win.

The use case for this is, I want to have a powerful <list> that have virtual scrolling, pagination, sorting, filtering, or anything. And to do that, I need to access/modify/overwrite it's children.

If we can pre-process the template, then we can rewrite it's structure and get it's children overwritten.

My point is the user does not need to know them to use my <list> component, they can just write the content of first item, and toggle some features by passing some props

@rizrmd
Copy link
Author

rizrmd commented Oct 22, 2017

Well I just did it in server side, before each component sent to client:

  1. Parse my html template
  2. Look for <list> component
  3. Pre-process it. Turn a <list>... into <list><div m-for="">...
  4. Send it to client (HTTP 200 OK)

Now, my <list> is working properly,

or not...

I don't know, what`s the catch ?

@kbrsh
Copy link
Owner

kbrsh commented Oct 22, 2017

I think it's a cool idea, but a little hacky. Instead of having it be preprocessed, I'd recommend a component with an API like:

<List>
  <div m-for="item in items"></div>
</List>

See, slots are bound to the parent, so the m-for data items will be coming from the parent. From here, the List component can use an <m-insert/> to insert the children, and create the list component like that.

You can manipulate the DOM as well. Each component (after being mounted) has access to this.root, which contains the DOM element bound to it.

@rizrmd
Copy link
Author

rizrmd commented Oct 22, 2017

Ah, I think this.root solves it!

@rizrmd rizrmd closed this as completed Oct 22, 2017
@rizrmd
Copy link
Author

rizrmd commented Oct 22, 2017

It would be nice if Moon gives a warning when user try to put multiple inserts

@kbrsh
Copy link
Owner

kbrsh commented Oct 22, 2017

Cool! I'll try and add a warning, so for now I can keep this issue open.

@kbrsh kbrsh reopened this Oct 22, 2017
@kbrsh
Copy link
Owner

kbrsh commented Oct 22, 2017

Updating this.root by appending, removing, or replacing elements will break the next render. So will setting any props on elements.

It's mostly so you can set stuff like the height and width of the element. You can also access the insert of the component and then render it using a custom render function.

@rizrmd
Copy link
Author

rizrmd commented Oct 22, 2017

Oops, sorry i just created new issue regarding that question.

@kbrsh
Copy link
Owner

kbrsh commented Nov 3, 2017

Adding a warning for multiple slots is actually more complex than I initially thought. It would require a getter to see if it has been accessed multiple times and would require code in the render function or code generator.

I don't think it's that big of an issue though, so I'm closing it for now. Let me know if you have any thoughts on this.

@kbrsh kbrsh closed this as completed Nov 3, 2017
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