[GH-ISSUE #718] Form: AddDropDown immediately calls SelectedFunc #523

Closed
opened 2026-03-04 01:05:44 +03:00 by kerem · 7 comments
Owner

Originally created by @moson-mo on GitHub (Apr 3, 2022).
Original GitHub issue: https://github.com/rivo/tview/issues/718

Hi,

When you add a new DropDown item to a form, the "selected" func is immediately executed. (because the handler is registered prior to the SetCurrentOption call)

This does not seem to happen for any of the other form items.

regards,
MO


Very nice lib btw. I love it 😉

Originally created by @moson-mo on GitHub (Apr 3, 2022). Original GitHub issue: https://github.com/rivo/tview/issues/718 Hi, When you add a new DropDown item to a form, the "selected" func is immediately executed. (because the handler is registered prior to the SetCurrentOption call) This does not seem to happen for any of the other form items. regards, MO --- Very nice lib btw. I love it :wink:
kerem closed this issue 2026-03-04 01:05:44 +03:00
Author
Owner

@rivo commented on GitHub (Apr 15, 2022):

This is on purpose because other parts of the application may depend on the current selection of this drop-down. I didn't want to force users to query the initial state of the drop-down themselves (which would various function calls including a cast to DropDown). You could set the initial drop-down item to -1 but even then, the selected function would be called (see the documentation).

<!-- gh-comment-id:1100289935 --> @rivo commented on GitHub (Apr 15, 2022): This is on purpose because other parts of the application may depend on the current selection of this drop-down. I didn't want to force users to query the initial state of the drop-down themselves (which would various function calls including a cast to `DropDown`). You could set the initial drop-down item to `-1` but even then, the `selected` function would be called (see the [documentation](https://pkg.go.dev/github.com/rivo/tview#DropDown.SetSelectedFunc)).
Author
Owner

@moson-mo commented on GitHub (Apr 15, 2022):

because other parts of the application may depend on the current selection of this drop-down

Hmm. That would also be valid for other fields like checkbox. That one, however, does not trigger change func because the value is set before the change func is registered. So it's somewhat inconsistent. 😉

<!-- gh-comment-id:1100295667 --> @moson-mo commented on GitHub (Apr 15, 2022): > because other parts of the application may depend on the current selection of this drop-down Hmm. That would also be valid for other fields like checkbox. That one, however, does not trigger change func because the value is set before the change func is registered. So it's somewhat inconsistent. 😉
Author
Owner

@rivo commented on GitHub (Apr 15, 2022):

You're right. It would also affect Checkbox and InputField. There is a difference between these two and DropDown, however. It might just be a semantic difference, but the DropDown event handler is called selected whereas the handlers of the other two are called changed. I guess one could argue that going from "uninitialized" to "initial selection" is a selection but not a change.

Ok, but we should be looking at this in a more pragmatic way. Does the current behaviour cause problems in your application?

<!-- gh-comment-id:1100303589 --> @rivo commented on GitHub (Apr 15, 2022): You're right. It would also affect `Checkbox` and `InputField`. There is a difference between these two and `DropDown`, however. It might just be a semantic difference, but the `DropDown` event handler is called `selected` whereas the handlers of the other two are called `changed`. I guess one could argue that going from "uninitialized" to "initial selection" is a selection but not a change. Ok, but we should be looking at this in a more pragmatic way. Does the current behaviour cause problems in your application?
Author
Owner

@moson-mo commented on GitHub (Apr 15, 2022):

Does the current behaviour cause problems in your application?

No, not really, I'm just working around this by registering the handler later on for those dropdown fields.

ps.settings.AddDropDown("Search by: ", []string{"Name", "Name & Description"}, by, nil)
	if dd, ok := ps.settings.GetFormItemByLabel("Search by: ").(*tview.DropDown); ok {
		dd.SetSelectedFunc(func(text string, index int) {
			if text != ps.conf.SearchBy {
				ps.settingsChanged = true
			}
		})
	}

That being said, I should probably change my approach entirely.

Right now I don't really care about the value itself when a setting / form item is changed. I just register that something has changed. And when the user hit's "Save", I'm collecting the values from all form items and write them back into my "config struct".

Instead of that, I should probably use a second temporary struct instead where any changes are written directly when the change/select func is called. And then compare the values of the fields in both structs to check if something has changed and needs to be saved...

<!-- gh-comment-id:1100309784 --> @moson-mo commented on GitHub (Apr 15, 2022): > Does the current behaviour cause problems in your application? No, not really, I'm just working around this by registering the handler later on for those dropdown fields. ``` ps.settings.AddDropDown("Search by: ", []string{"Name", "Name & Description"}, by, nil) if dd, ok := ps.settings.GetFormItemByLabel("Search by: ").(*tview.DropDown); ok { dd.SetSelectedFunc(func(text string, index int) { if text != ps.conf.SearchBy { ps.settingsChanged = true } }) } ``` That being said, I should probably change my approach entirely. Right now I don't really care about the value itself when a setting / form item is changed. I just register that something has changed. And when the user hit's "Save", I'm collecting the values from all form items and write them back into my "config struct". Instead of that, I should probably use a second temporary struct instead where any changes are written directly when the change/select func is called. And then compare the values of the fields in both structs to check if something has changed and needs to be saved...
Author
Owner

@moson-mo commented on GitHub (Apr 15, 2022):

I see your point, change vs. select and also don't want to break things for other applications relying on the current behaviour hence I closed the pull request.

Would be cool if you can reference "pacseek" in the list of apps though (https://github.com/rivo/tview/pull/719/commits/53cae823c28d294a7efb5617985bc099abd6ca93)

Note that I did not intend to sneak that thing in with the pull. I'm just bad dealing with git 😉😂

<!-- gh-comment-id:1100337666 --> @moson-mo commented on GitHub (Apr 15, 2022): I see your point, change vs. select and also don't want to break things for other applications relying on the current behaviour hence I closed the pull request. Would be cool if you can reference "pacseek" in the list of apps though (https://github.com/rivo/tview/pull/719/commits/53cae823c28d294a7efb5617985bc099abd6ca93) Note that I did not intend to sneak that thing in with the pull. I'm just bad dealing with git 😉😂
Author
Owner

@rivo commented on GitHub (Apr 15, 2022):

No problem. Can you submit a pull request for this commit?

<!-- gh-comment-id:1100363970 --> @rivo commented on GitHub (Apr 15, 2022): No problem. Can you submit a pull request for this commit?
Author
Owner

@moson-mo commented on GitHub (Apr 15, 2022):

https://github.com/rivo/tview/pull/722

Thanks a lot.

<!-- gh-comment-id:1100375533 --> @moson-mo commented on GitHub (Apr 15, 2022): https://github.com/rivo/tview/pull/722 Thanks a lot.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/tview#523
No description provided.