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

Regression: children aren't displayed when appended manually to a parent with no child. #24

Closed
ArthurSonzogni opened this issue May 31, 2019 · 3 comments

Comments

@ArthurSonzogni
Copy link
Contributor

I discovered my example:
https://github.com/ArthurSonzogni/asm-dom-starter/blob/master/src/index.cpp

doesn't work anymore on version 0.6.2

Here is the relevant snippet:

  // Limit the number of actions to 10.
  asmdom::VNode *action_list = <ul class = "action_list"></ ul>;
  int start = std::max(0, (int)actions.size() - 10);
  for (int i = start; i < actions.size(); ++i) {
    std::string label = "action[" + std::to_string(i) + "] = " + actions[i];
    action_list->children.push_back(<li>{label}</ li>);
  }

I tried adding one child in the initial action_list element. Then my manually added children are properly displayed.

It looks like a regression. Or maybe I was using it the wrong way.

@mbasso
Copy link
Owner

mbasso commented Jun 1, 2019

Hi @ArthurSonzogni,
in the latest version of asm-dom a normalization of VNodes is executed immediately after their creation. So, in this case new children are simply ignored... It will continue to work after the merge and release of PR #21, since the normalization will take place only when really needed (when patch is called).
However, adding children after the creation of a VNode is not recommended, it should be created using CPX or the h function and then never modified by the user.
With CPX you can set children dynamically in the way defined here, using asmdom::Children.

I think we can leave this issue open and update the doc to highlight it. What do you think?
Let me know if it works fine after that little change 😄

@ArthurSonzogni
Copy link
Contributor Author

Yes, I am now using it with:

asmdom::Children children
for(auto it = ..)
  children.push_back(it)

return <div> {...children} </div>

Which looks better to me.

Feel free to close this bug. I just observed something regressed, but not supporting this looks good to me.

@mbasso
Copy link
Owner

mbasso commented Jun 4, 2019

Perfect! I'll close this

@mbasso mbasso closed this as completed Jun 4, 2019
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