Add value property to radiomenuitem

[ctadlock]

Need to be able to set properties["tornadofx.toggleGroupValue"] so can bind to ToggleGroup SelectedValue.

[edvin]

Can you explain what you're trying to do?

[edvin]

Here is a small example View with two buttons inside a toggle group, and a listener on the selected value of the group:

class MyView : View() {
    override val root = hbox {
        togglegroup {
            togglebutton("One", value = 1)
            togglebutton("Two", value = 2)
            selectedValueProperty<Int>().onChange {
                println("Value is $it")
            }
        }
    }
}

[ctadlock]

Implement a language chooser. The property DesktopApp.instance.language will set FX.locale and save the value in config.

[edvin]

If I understand correctly, you need a way to assign a value to a checkbox menu item, like you can do for the togglebutton, right? I'll implement this later today :)

[ctadlock]

Correct, need to be able to set radiomenuitem.value in the builder. Im doing it now by just manually setting the internal property properties["tornadofx.toggleGroupValue"].

Everything else look OK in this code? There arent any good examples of using radiomenuitems or togglegroups in menus that I can find online.

Thanks for the help!

[edvin]

Yeah, the rest looks good - you've grasped the essence of the framework nicely :)) Will get back to you later today.

[edvin]

PS: In this particular case you probably don't gain anything from wrapping the selectedLanguageProperty in a model, it might as well be a field member in the View class :) You could even just listen for changes to the selected value property of the toggle group directly, but I suspect you're aware of this and chose that approach for a reason. Just want to mention it in case others read this.

[ctadlock]

Thanks for the tip. There isnt a lot of guidance on that subject of when to use a ViewModel and when to just have a property in the View. It seems ViewModels are more appropriate for forms that need to have data in a working state before being committed.

In my specific case I already have a property in DesktopApp so I just bound to that instead.

[ctadlock]

Since I have you... ;) What is the best way to reload the app with the new locale set? I was just going to go old school and restart the app. Is there a better way?

[edvin]

Sorry for not getting back to you yesterday, stuff happened :) Trying again today.

[edvin]

I've added the value property to all radiomenuitem builders :)

[edvin]

There isn't an easy way to reload the app when the locale changes. Any text retrieved from the resource bundles are already statically set. We could fix this by exposing observable properties for the resource bundle lookups, but I don't think it's worth it.

[ctadlock]

@edvin I think there is an issue(s) with the implementation of this. Either that or Im not understanding how this is supposed to work. Sadly, Im just now getting back to this code.

1. The name property should be used for the value of tornadofx.toggleGroupValue instead of text. I noticed in your commit that in 2 places you used text and one place you used name. (677ed52)
2. properties["tornadofx.toggleGroupValue"] is being set on the wrong object. It should be set on it which is the RadioMenuItem

My re-implementation of Menu.radiomenuitemX works as Id expect it to.

Please advice on if this is a bug or my misunderstanding.

class TestWindow : View("test") {

    val testProperty: StringProperty = SimpleStringProperty()
    var test: String by testProperty

    override val root = vbox {
        menubar {
            menu("App") {
                menu("Test") {
                    togglegroup {
                        radiomenuitemX(
                                name = "apple",
                                toggleGroup = this)
                        radiomenuitemX(
                                name = "bear",
                                toggleGroup = this)
                        radiomenuitemX(
                                name = "cat",
                                toggleGroup = this)
                        bind(testProperty)
                    }
                }
            }
        }
    }

    init {
        testProperty.onChange {
            testProperty.value?.apply {
                println("test is: $it")
            }
        }

        testProperty.value = "apple"
    }
}

fun Menu.radiomenuitemX(
        name: String, toggleGroup: ToggleGroup? = null, keyCombination: KeyCombination? = null,
        graphic: Node? = null, value: Any? = null, op: RadioMenuItem.() -> Unit = {}
)  = RadioMenuItem(name, graphic).also {
    toggleGroup?.apply { it.toggleGroup = this }
    keyCombination?.apply { it.accelerator = this }
    // properties["tornadofx.toggleGroupValue"] = value ?: text // current, doesnt work
    it.properties["tornadofx.toggleGroupValue"] = value ?: name
    graphic?.apply { it.graphic = graphic }
    op(it)
    this += it
}

[edvin]

You're right, sorry about that. I've committed a fix now.