Add dropdown

[jrmoulton]

@pieterdd

I'm marking this as a draft because I still need to add docs / examples and it'd be awesome if @pieterdd / anyone else could give some feedback on it.

This has an issue with styling being removed from the overlay when the overlay receives events.

sort of example:

dropdown(
    // pass in iterator of views ( this becomes the list in the dropdown)
    Iterator().map(move |val| {
        view(val).on_click_stop(move |_| signal.set())
    }),
    // View for the active item (this is the always visible part of the dropdown)
    |val| {
        stack((
            label(move || item),
            chevron_down_icon_view,
        ))
        .style(|s| s.items_center().justify_between().size_full())
    },
    // Signal change for the active item
    move ||signal.get(),
)

Like other widgets this doesn't automatically set the active item when an item in the list is clicked. Currently I am adding an on click handle to each item in the list. Feedback on this api would be nice.

[jrmoulton]

Another issue is that the overlay doesn't maintain relative position when the window is resized

[pieterdd]

I'm about to write a wall of text specifically about the API, so I'll focus exclusively on that. Is there a specific rationale for choosing move || signal.get() over signal? Most existing widgets seem to take a ReadSignal directly.

Although not writing to the signal internally is consistent with what most widgets do, text_input currently does take a RwSignal. For completeness sake, this is what that could look like for dropdown:

dropdown(
    a_list.iter(),
    |item| {
        stack((
            label(move || item),
            chevron_down_icon_view,
        ))
        .style(|s| s.items_center().justify_between().size_full())
    },
    rw_signal,
)

It's a slightly simpler, but more restrictive API.

If my memory serves me well, several UI frameworks including Vue follow a "data bubbles down, events bubble up" strategy where the UI component merely reads the variable and emits an event to indicate changes by the user. It seems like a good principle, and your proposal seems consistent with that, so RwSignal is my least preferred option for now. In fact, I wouldn't mind if text_input changed over to using a ReadSignal as well.

I've also been contemplating the implications of having an on_change event callback. Concept:

dropdown(
    a_list.iter(),
    |item| {
        stack((
            label(move || item),
            chevron_down_icon_view,
        ))
        .style(|s| s.items_center().justify_between().size_full())
    },
    signal,
).on_change(move |_, new_value| set_signal.set(new_value))

This feels like a bit of a compromise between the flexibility of letting the caller control the event handler and the simplicity of keeping the lower level stuff behind the scenes. Currently I quite like this, although I haven't sufficiently considered possible downsides to voice a definitive opinion about it.

So to recap, the options I see:

• ReadSignal with explicit low-level click handler (signal or move || signal.get())
• RwSignal with implicit signal value update
• ReadSignal with explicit high-level change handler

(Disclaimer: I haven't tested any of the snippets, just making this up as I go along)

[jrmoulton]

Another thing to be added is a function to drive the open/close state of the list

Edit: implemented

[jrmoulton]

The issues on this pr around the styling of overlays still exists but that is separate from the dropdown.

The issue of API for on_updates still applies but that is also a larger problem and stems more from the list API.

The example could be improved for consistency / isn't the prettiest but it is functional

[jrmoulton]

@pieterdd I want the on_change api for this but I took a stab at implementing it and couldn't get it to work

[pieterdd]

@jrmoulton I couldn't implement a working widget_func(...).on_change() API either. I've suggested this code change as a backup plan. It's not ideal from a user perspective but it's easy to maintain, so I'm putting it on the table as an option.

[jrmoulton]

I'm open for any feedback @dzhou121 but I think this is good to merge for now

[dominikwilkowski]

Can we fix up the SVG markup a bit:

const CHEVRON_DOWN: &str = r#<svg xmlns="http://www.w3.org/2000/svg" xml:space="preserve" viewBox="0 0 185.344 185.344">
  <path fill="#010002" d="M92.672 144.373a10.707 10.707 0 0 1-7.593-3.138L3.145 59.301c-4.194-4.199-4.194-10.992 0-15.18a10.72 10.72 0 0 1 15.18 0l74.347 74.341 74.347-74.341a10.72 10.72 0 0 1 15.18 0c4.194 4.194 4.194 10.981 0 15.18l-81.939 81.934a10.694 10.694 0 0 1-7.588 3.138z"/>
</svg>"#;

Note I haven't tested the SVG in your code, only the SVG code itself

[presiyan-ivanov]

It seems like a good principle, and your proposal seems consistent with that, so RwSignal is my least preferred option for now. In fact, I wouldn't mind if text_input changed over to using a ReadSignal as well.

Just started checking the recent discussions on the on_update topic and I wanted to say I don't like that input is using RwSignal - it is not a good example and imo using RwSignal should be avoided. It was done like this because I wanted to get something done quickly(and this felt a quicker way to "get it done") and it stayed like this.
I have been planning to do that myself for some time actually, but I was on the fence, since the input will be replaced with lapce's input anyway. Also, I did a small refactor on the input state handling on my local, which should make changing it easier, so I may look into changing it to a ReadSignal after my local changes are merged.

[jrmoulton]