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.

Sign up for GitHub

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

Jump to bottom

buildHtml to take multiple elements #7

Closed
bluenote10 opened this issue May 14, 2017 · 9 comments
Closed

buildHtml to take multiple elements #7

bluenote10 opened this issue May 14, 2017 · 9 comments
Labels
enhancement

Comments

@bluenote10
Copy link
Contributor

Not sure if this is by design, but having multiple top level elements in buildHtml does not seem to work:

proc createDom(): VNode =
  result = buildHtml():
    tdiv():
      text "test"
    tdiv():
      text "test"

Leads to: Error: expression 'tmp94011' is of type 'VNode' and has to be discarded

Wrapping them into one top level element solve the problem.
@Araq
Copy link
Collaborator

Araq commented May 14, 2017

Huh? Pretty sure I have code like this.

@bluenote10
Copy link
Contributor Author

bluenote10 commented May 14, 2017 •
edited

Hm, okay not sure what could be different. Here's my full example to reproduce:

include karaxprelude

proc renderer(): VNode =
  result = buildHtml():
    tdiv():
      text "test"
    tdiv():
      text "test"

setRenderer renderer

Running with updated karax & nim devel branches this afternoon. And the output of running with -d:debugKaraxDsl:

  let tmp67004 = tree(VNodeKind.tdiv)
  add(tmp67004, text "test")
  tmp67004
  let tmp67005 = tree(VNodeKind.tdiv)
  add(tmp67005, text "test")
  tmp67005
reproduce.nim(4, 21) template/generic instantiation from here
/home/fabian/.nimble/pkgs/karax-0.1.0/karaxdsl.nim(62, 23) Error: expression 'tmp67011' is of type 'VNode' and has to be discarded

Strange that the symbol is not actually a symbol of the macro.

@Araq
Copy link
Collaborator

Araq commented May 15, 2017 •
edited

Oh yeah, well that can't work, there is no root node to attach the 2 divs to. Maybe the buildHtml that only takes children is a bad idea. :-)

@bluenote10
Copy link
Contributor Author

bluenote10 commented May 15, 2017 •
edited

Shouldn't there be the main/global entry point of the app <div id="ROOT"></div>?

A use case for a "multiple children buildHtml" would be:

proc renderList(): VNode =
  # don't want to make a decision about the container here
  result = buildHtml():
    tdiv():
      text "child 1"
    tdiv():
      text "child 2"

proc renderer(): VNode =
  result = buildHtml():
    tdiv():
      renderList()
    aDifferentContainer():
      renderList()

This is by the way one of the limitations in Vue (each component must have a single root) and often results in wrapping things in slightly annoying dummy divs.

@Araq
Copy link
Collaborator

Araq commented Jun 22, 2017

This is by the way one of the limitations in Vue (each component must have a single root) and often results in wrapping things in slightly annoying dummy divs.

We can perhaps introdue a special VNodeKind.unnest that is embedded into the parent node, but this is more complex than it sounds. :-(

@bluenote10
Copy link
Contributor Author

Yes, this is probably not so easy. The question is whether a VComponent should allow for unnested nodes as well, which looks complicated to me. I think it would be an acceptable compromise the require VComponents to be nested, but solve the problem at least for buildHtml to make factoring out view elements easier. I had two ideas to solve this, but I didn't think them through yet.

The first idea was to fully switch to lists of nodes, because html itself is always a list of nodes within <body></body>. This would mean for instance to define type VNodes: seq[VNode], change the signature to buildHtml(): VNodes, and the renderer would return VNodes. Whenever the karaxdsl comes across a VNodes element it iterates over it and appends all children. Not sure how feasible this is though.

The other idea is to stay with a single VNode return type, but make it easier to pass the context (or parent/currentNode) to sub-view elements. If the karaxdsl would inject a name like currentNode exposing the context, the example from above could be solved like that:

proc renderList(parent: VNode): VNode =
  # don't want to make a decision about the container here
  # solved by forwarding the parent to buildHtml
  result = buildHtml(parent):
    tdiv():
      text "child 1"
    tdiv():
      text "child 2"

proc renderer(): VNode =
  result = buildHtml():
    tdiv():
      renderList(currentNode)
    aDifferentContainer():
      renderList(currentNode)

This might clash (or rather confuse people) with the current overload of buildHtml to support a tag as its first parameter. Personally have never seen a big need for this, at least not, if the following two are equivalent anyway:

# Writing the root tag in the body looks good to me
result = buildHtml():
  tdiv(id="A"):
    tdiv(id="sub"):
      text "a"

# Do we need support for this, any benefit?
result = buildHtml(tdiv(id="A")):
  tdiv(id="sub"):
    text "a"

This second solution should work fine for inner levels -- on the outermost level it could be tricky if it requires e.g. a dummy parent.

@bluenote10
Copy link
Contributor Author

FYI: Inspired by Karax, I wrote a similar DSL to generate SVG files/animations. As an experiment I modified the DSL in a way that the main buildSvg macro returns a seq[Node] making it possible to handle flat structures in general. Maybe this would work for Karax as well? If you are interested here is the macro.

@sclee15
Copy link

sclee15 commented Feb 27, 2018

I second this feature.

@geotre
Copy link
Collaborator

geotre commented Mar 2, 2023

I'm going to close this for now due to inactivity. It sounds like this may be able to be supported in the future, but is not currently. Please feel free to re-open and comment if you want to discuss further!

@geotre geotre closed this as completed Mar 2, 2023
@geotre geotre closed this as not planned Won't fix, can't repro, duplicate, stale Mar 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@Araq @geotre @bluenote10 @ajusa @sclee15