Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Listbox component to handle nullables #758

Open
Inkemann opened this issue Mar 9, 2023 · 3 comments
Open

Allow Listbox component to handle nullables #758

Inkemann opened this issue Mar 9, 2023 · 3 comments
Labels
discussion Extra attention is needed headless All about headless components and foundations

Comments

@Inkemann
Copy link

Inkemann commented Mar 9, 2023

Currently the Listbox component doesn't play well with nullable types, due to the way the ListboxEntries are handled (an entry with a value of null is treated the same a disabled entry and therefore ignored when clicked). In my opinion a nullable selection (that contains a "None" option at the end of the list of entries for example) is a very common use case though and should work with the component out of the box.
Or is this a deliberate design choice (and if so, why)?

@ghost
Copy link

ghost commented Mar 9, 2023

It is a design choice. I am only on mobile currently and also need some time to craft an understandable answer. So please stay tuned 😉

@ghost
Copy link

ghost commented Mar 10, 2023

So the headless idea is to provide a common base of functionality for typical UI elements.

A select menu has as base the "select 1 from many" semantic. But thinking about variants, three behaviors can be derived by this:

  1. "There is a default option selected and the user can change this. In the end there is always one option selected"
  2. "Initially there is no option selected, but the user has to choose one, so once opened the box, one item will be selected in the end"
  3. "The selection is totally optional and can also be reseted to no selection"

In order to support all three semantics, we have to be strict at the headless level. Other ways we could not support use case one.

That said, you are totally free to model the other two cases on top of our headless selectMenu encapsulating that in your crafted component. We have done so in some customer UI framework we have developed on top of our headless components.

Does this answer your question?

@Inkemann
Copy link
Author

Inkemann commented Mar 10, 2023

But wouldn't full support of nullable types achieve all three use-cases for the component? 1. is fulfilled when a non-nullable type is provided (like in the Star Wars example for the component). 2. is fulfilled when a nullable type is provided with a default of null (which works fine with the existing component) and no null entry is added. 3. could be fulfilled by simply allowing a null entry to be selected (which is the only thing that does not work right now even though the entry is rendered correctly). Either way, I think the documentation should be more explicit about this, because since nullable types are permitted it is easy to come to the false conclusion that the desired behaviour I outlined in my original post is provided out of the box.

Anyway, an easy solution to achieve my desired behaviour with the built-in listbox was to wrap the nullable type in a non-nullable type with a nullable property and use a very simple lens for mapping the value to a store or the like.
So instead of doing this:

val someTypeList = listOf(someTypeA, someTypeB, null)
val someTypeStore = storeOf<SomeType?>(null)

listbox<SomeType?> {
    value(someTypeStore)
    listboxItems {
        someTypeList.forEach {
            listboxItem(it) {
                // Render some content
            }
        }
    }
}

I did this:

val someTypeList = listOf(someTypeA, someTypeB, null)
val someTypeStore = storeOf<SomeType?>(null)

data class SomeTypeWrapper(val someType: SomeType?)

val lens = lensOf<SomeType?, SomeTypeWrapper>("", { SomeTypeWrapper(it) }, { _, it -> it.someType})

listbox<SomeTypeWrapper> {
    value(someTypeStore.map(lens))
    listboxItems {
        someTypeList.map { SomeTypeWrapper(it) }.forEach {
            listboxItem(it) {
                // Render some content
            }
        }
    }
}

@Lysander Lysander added headless All about headless components and foundations discussion Extra attention is needed labels Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Extra attention is needed headless All about headless components and foundations
Projects
None yet
Development

No branches or pull requests

2 participants