karaxnim / karax

Karax. Single page applications for Nim.
MIT License
1.07k stars 90 forks source link

Karax does not update event handlers #267

Closed hamidb80 closed 1 year ago

hamidb80 commented 1 year ago

as you can see, there is a simple todo app with the ability to remove tasks. everything works until I delete some ...

https://github.com/karaxnim/karax/assets/33871336/36b73278-9dd5-400a-818e-1127f3fcb819

index.html

<!DOCTYPE html>
<html lang="en">

<head>
  <meta charset="UTF-8">
  <meta http-equiv="X-UA-Compatible" content="IE=edge">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>Karax Closure Fn Test</title>
  <link rel="stylesheet" href="https://bootswatch.com/5/flatly/bootstrap.min.css">
  <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/bootstrap-icons@1.10.5/font/bootstrap-icons.css">
  <script src="./script.js" defer></script>
</head>

<body>
  <div id="ROOT" class="w-100"></div>
</body>
</html>

script.nim

include karax/prelude

var tasks: seq[string]

proc createDom(): VNode =
  result = buildHtml tdiv:

    tdiv(class="d-grid gap-2 p-2"):
      button(class="btn btn-primary"):
        italic(class="bi bi-plus-square-fill mx-2")
        text "add"

      proc onclick = 
        tasks.add "task number: " & $tasks.len

    ul(class = "list-group"):
      for i, t in tasks:
        li(class = "list-group-item list-group-item-action d-flex justify-content-between align-items-center"):
          span():
            text t

          tdiv:
            button(class = "btn btn-outline-primary btn-small rounded-pill"):
              italic(class="bi bi-caret-down-fill mx-2")

              proc onclick = 
                echo "down ", i
                swap tasks[i], tasks[i+1]

            button(class = "btn btn-outline-primary btn-small rounded-pill mx-2"):
              italic(class="bi bi-caret-up-fill mx-2")

              proc onclick = 
                echo "up ", i
                swap tasks[i], tasks[i-1]

            button(class = "btn btn-outline-primary btn-small rounded-pill mx-2"):
              italic(class="bi bi-trash-fill mx-2")

              proc onclick = 
                echo "deleted ", i
                delete tasks, i

setRenderer createDom
Araq commented 1 year ago

Haven't tried it recently but back then when I developed it, deleting worked, can you believe it. So it's a regression.

geotre commented 1 year ago

I don't think this is a regression, there has always been a problem capturing in event handlers. I can replicate the above problem with ~4 year old versions of Karax (e.g. commit c091af16, picked randomly from 2019)

Two solutions are:

  1. Use properties of the vnode, similar to the todoapp example:

import std/sugar
include karax/prelude

var tasks: seq[string]

proc createDom(): VNode =
  result = buildHtml tdiv:

    tdiv(class="d-grid gap-2 p-2"):
      button(class="btn btn-primary"):
        italic(class="bi bi-plus-square-fill mx-2")
        text "add"

      proc onclick = 
        tasks.add "task number: " & $tasks.len

    ul(class = "list-group"):
      for i, t in tasks:
        li(class = "list-group-item list-group-item-action d-flex justify-content-between align-items-center"):
          span():
            text t

          tdiv:
            button(index=i, class = "btn btn-outline-primary btn-small rounded-pill"):
              italic(class="bi bi-caret-down-fill mx-2")

              proc onclick(ev: Event; n: VNode) = 
                echo "down ", n.index
                swap tasks[n.index], tasks[n.index+1]

            button(index=i, class = "btn btn-outline-primary btn-small rounded-pill mx-2"):
              italic(class="bi bi-caret-up-fill mx-2")

              proc onclick(ev: Event; n: VNode) = 
                echo "up ", n.index
                swap tasks[n.index], tasks[n.index-1]

            button(index=i, class = "btn btn-outline-primary btn-small rounded-pill mx-2"):
              italic(class="bi bi-trash-fill mx-2")
              proc onclick(ev: Event; n: VNode) = 
                  echo "deleted ", n.index
                  delete tasks, n.index

setRenderer createDom
  1. Generate callbacks:
include karax/prelude

var tasks: seq[string]

proc makeUpCb(i: int): auto =
  return proc(ev: Event; n: VNode) =
    echo "up ", i
    swap tasks[i], tasks[i-1]

proc makeDownCb(i: int): auto =
  return proc(ev: Event; n: VNode) =
    echo "down ", i
    swap tasks[i], tasks[i+1]

proc makeDeleteCb(i: int): auto =
  return proc(ev: Event; n: VNode) =
    echo "deleted ", i
    delete tasks, i

proc createDom(): VNode =
  result = buildHtml tdiv:

    tdiv(class="d-grid gap-2 p-2"):
      button(class="btn btn-primary"):
        italic(class="bi bi-plus-square-fill mx-2")
        text "add"

      proc onclick = 
        tasks.add "task number: " & $tasks.len

    ul(class = "list-group"):
      for i, t in tasks:
        li(class = "list-group-item list-group-item-action d-flex justify-content-between align-items-center"):
          span():
            text t

          tdiv:
            button(onclick=makeDownCb(i), class = "btn btn-outline-primary btn-small rounded-pill"):
              italic(class="bi bi-caret-down-fill mx-2")

            button(onclick=makeUpCb(i), class = "btn btn-outline-primary btn-small rounded-pill mx-2"):
              italic(class="bi bi-caret-up-fill mx-2")

            button(onclick = makeDeleteCb(i), class = "btn btn-outline-primary btn-small rounded-pill mx-2"):
              italic(class="bi bi-trash-fill mx-2")

setRenderer createDom
hamidb80 commented 1 year ago

that's interesting! I didn't know about index attribute,

Thanks!

geotre commented 1 year ago

No problem. I'll close the issue for now. Feel free to reopen if you want to discuss further