rjaros / kvision

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

TomSelectRemote: preload option behavior undesirable #539

Closed drammelt closed 1 month ago

drammelt commented 1 month ago

When using TomSelectRemote the preload option breaks loading of subsequent requests.

        val loadCallback: ((query: String, callback: (Array<dynamic>) -> Unit) -> Unit)? = if (!preload) {
            { query, callback ->
                tomSelectJs?.clearOptions()
                loadResults(callAgent, url, method, query, this.value, requestFilter, callback)
            }
        } else null < this breaks subsequent loads

https://github.com/rjaros/kvision/blob/41b07b48f5386afe34eaf8fb3da361fb12d498b7/kvision-modules/kvision-tom-select-remote/src/jsMain/kotlin/io/kvision/form/select/TomSelectRemoteInput.kt#L74C1-L79C20

The option as it is seems to be concluding that the list would be a set list called at init and never changed. If this was a desired behavior it would be better to change preload to an enum as it essentially is in the TomSelect js.

In js you can set, preload = true/false or "focus", an additional option could be added "once" or similar.

So we would have

enum PRELOAD_OPTIONS {
  "OFF", // or false?
  "ON",
  "FOCUS",
  "ONCE"
}
rjaros commented 1 month ago

Hello,

The "focus" value for preloading can be used with preloadOnFocus property of the tsOptions: TomSelectOptions? parameter. Initially when this component was implemented the "focus" value was not supported at all, and later it was added in a way to not break any existing code. You are right, using an enum would be best solution, but now it have to wait for the next major release.

drammelt commented 1 month ago

Ok but how can I get the correct behavior for preload which is to load the data when the TomSelectRemote is inserted then process additional load calls when something is typed in the input?

rjaros commented 1 month ago

As you have correctly noticed, the preload property of TomSelectRemote is used to create the list at the initialization time and never change it (it is designed to effectively disable the load callback).

But you can also use the preload option within the tsOptions parameter, and it will work with the default behaviour of the TomSelect JS component. Together with the preloadOnFocus option I've mentioned earlier, you can set all three values supported by the component (false, true and focus).

drammelt commented 1 month ago

OK that is perfect thank you, I will use the tsOptions for now.

Thank you for your help again.

drammelt commented 1 month ago

Just incase anyone else has this issue... setting the TomSelectRemote tsOptions does nothing to solve this...

tomSelectRemote(
                label = "Test",
                serviceManager = getServiceManager(),
                function = IFrontEndService::filterData,
                tsOptions = TomSelectOptions(
                    preloadOnFocus = true,
                    preload = true,
                    openOnFocus = true,
                ),
            ) {
                placeholder = "Begin typing stuff"
            }

Same behavior as normal.

You have to set tomSelectRemote.input.tsOptions

tomSelectRemote(
                label = "Test",
                serviceManager = getServiceManager(),
                function = IFrontEndService::filterData,
            ) {
                this.input.tsOptions = TomSelectOptions(
                    preloadOnFocus = true,
                    preload = true,
                    openOnFocus = true,
                )
                placeholder = "Begin typing stuff"
            }

This works.

rjaros commented 1 month ago

This looks like a bug. I'll check that.

drammelt commented 1 month ago

Test app here if it helps

https://github.com/drammelt/kvision-tomselecttest

rjaros commented 1 month ago

Fixed in 8.0.1