rjaros / kvision

Object oriented web framework for Kotlin/JS
https://kvision.io
MIT License
1.2k stars 66 forks source link

StackPanel seems to not indexing components properly #508

Closed CodeServant closed 5 months ago

CodeServant commented 5 months ago

Hi, I am learning Kvision and going through some examples. I've tried to recreate switching between panels behaviour, so i found stackPanel which seems ideal for this.
I created this code at the end of hPanel in showcase program in Showcase.kt just below themeSwitcher:

val txChild = Text(floating = true, label = "this is label")

val btn = Button(text = "Click me!")

val index = ObservableValue(0) // 0  - show button first
btn.bind(index) {
    onClick{
        index.value = 1
    }
}
txChild.value = "some"
stackPanel().bind(index) {
    // this not working
    add(btn)
    add(txChild)

    //this works perfeclly
    /*button(text = "Click me!") {
        onClick {
            index.value = 1
        }
    }
    text(floating = true, label = "this is label")*/
    activeIndex = it
}
span().bind(index) {
    +"${it}"
}

It seems like the stackPanel isn't indexing add(txChild) that is, I can't activate if via passing index. The goal of this program is to change button for the text when clicking on this button. The commented code works perfectly but not the code higher.

Am I doing something wrong? I started to belive this is bug, so I am writing it here.

rjaros commented 5 months ago

Hello,

When using state binding all children elements of the container (the stack panel) are removed and disposed on every state change. In your case you are trying to reuse the same instances of button and text. But after first click the Text component is disposed and can't be used (it's still there but rendered as an empty div).

The correct way to work with state binding is using DSL. In this case the children components are disposed too, but they are also recreated (usually with a new state).

CodeServant commented 5 months ago

So, If i want to switch between panels via reference to the Component it should be like this. Thats works for me. So it is officialy correct?

data class StackPanelBindings(
    val tx: Text,
    val btn: Button
)

val index = ObservableValue(
    StackPanelBindings(
        Text(floating = true, label = "this is label"),
        Button(text = "Click me!")
    ) 
)

stackPanel().bind(
    index
) {binding ->
    add(binding.btn)
    add(binding.tx)

    binding.btn.onClick {
        activeChild = binding.tx
    }
    activeChild = binding.btn
}
rjaros commented 5 months ago

No. I wouldn't call it correct. You need to ask yourself if (and why) you need instances of Text and Button components created manually. In general it should not be required, in fact you don't even need stack panel for your goal. You can just do this:

            val index = ObservableValue(0) // 0  - show button first

            simplePanel().bind(index) {
                if (it == 0) {
                    button(text = "Click me!") {
                        onClick {
                            index.value = 1
                        }
                    }
                } else {
                    text(floating = true, label = "this is label")
                }
            }
            span().bind(index) {
                +"${it}"
            }

If you want to access internal instances from outside you can declare a variable:

            val index = ObservableValue(0) // 0  - show button first

            var text: Text? = null
            simplePanel().bind(index) {
                if (it == 0) {
                    button(text = "Click me!") {
                        onClick {
                            index.value = 1
                        }
                    }
                } else {
                    text = text(floating = true, label = "this is label")
                }
            }
            span().bind(index) {
                +"${it}"
            }
            button("set new value") {
                onClick {
                    text?.value = "new value"
                }
            }

Still there are cases when externally defined components can be useful. But for such cases you should not use state binding, because you don't want to dispose and recreate you components. You just want to switch which is visible, and StackPanel is the right tool for this - just change activeIndex property and it will work.

            val index = ObservableValue(0) // 0  - show button first

            val txChild = Text(floating = true, label = "this is label")
            val btn = Button(text = "Click me!") {
                onClick {
                    index.value = 1
                }
            }

            stackPanel {
                add(btn)
                add(txChild)
                index.subscribe {
                    activeIndex = it
                }
            }

            span().bind(index) {
                +"${it}"
            }
CodeServant commented 5 months ago

ok I think I got this