reflex-dev / reflex

🕸️ Web apps in pure Python 🐍
https://reflex.dev
Apache License 2.0
20.27k stars 1.16k forks source link

Chakra multi-select does not work #3067

Closed TimChild closed 5 months ago

TimChild commented 7 months ago

Describe the bug The chakra multi select doesn't seem to do any multi selecting... The problem is visible directly in the docs here https://reflex.dev/docs/library/chakra/forms/select/

To Reproduce Same as example in docs

Expected behavior Expect to be able to select multiple options and that the dropdown stays open on selection. Instead it acts as a regular single select.

Specifics (please complete the following information):

Additional context

Probably it doesn't make sense to fix this component, and instead just remove the multi-select demo from the docs since reflex is moving away from chakra anyway.

It would be very nice to get some sort of multi-select somehow, but it's unfortunate that radix doesn't support it directly.

Also, I see this has been an issue before #1791 and PR #1861. Just linking these in case it's helpful.

I'll put a little effort into wrapping another react component and update if I get something working.

TimChild commented 7 months ago

I was able to wrap the react-select component without too much trouble, although it's not perfect.

Here's a minimal example

import reflex as rx
from typing import Any

from reflex.components.component import Component

class MultiSelectComponent(Component):
    library = "react-select"
    tag = "Select"
    is_default = True
    value: rx.Var[list[str]] = []
    options: rx.Var[list[dict[str, str]]] = []
    is_multi: rx.Var[bool] = True

    def get_event_triggers(self) -> dict[str, Any]:
        return {
            **super().get_event_triggers(),
            "on_change": lambda e0: [e0],
        }

multiselect = MultiSelectComponent.create

class MultiSelectState(rx.State):
    selected: list[dict[str, str]] = []

    def handle_change(self, change: list[dict[str, str]]):
        print(f"Change: {change}")
        self.selected = change

    @rx.cached_var
    def selected_values(self) -> str:
        print(self.selected)
        return ', '.join([d['value'] for d in self.selected])

@rx.page(route="/multi_select", title="Multi Select")
def index() -> rx.Component:
    return rx.box(
        multiselect(
            options=[{'value': 'opt1', 'label': 'Option 1'}, {'value': 'opt2', 'label': 'Option 2'}],
            value=MultiSelectState.selected,
            on_change=MultiSelectState.handle_change
        ),
    )

As it is currently implemented, the MultiSelectState and associated event handlers are required to make the component function properly.

Things to improve:

I'm not sure how to implement those improvements. But I'll leave this here anyway in case the version above is still useful to someone.

TimChild commented 7 months ago

To address making the component standalone, I think the rx.state.ComponentState is probably relevant, at least for holding the selected options. I'm still not sure about how to build in the necessary on_change update of the selected value though.

Or maybe something like the suggestion here effectively making something that behaves like a frontend state... https://github.com/reflex-dev/reflex/issues/1482#issuecomment-1866775511

TimChild commented 7 months ago

@masenf, have I used the rx.ComponentState correctly here? Specifically, the handling of the on_change event such that the component behaves correctly without having to use an external state, but also so that an external state can access the value changes if desired?

# multi_select_component.py
from typing import Any

import reflex as rx

class MultiSelectComponent(rx.Component):
    library = "react-select"
    tag = "Select"
    is_default = True
    value: rx.Var[list[str]] = []
    options: rx.Var[list[dict[str, str]]] = []
    is_multi: rx.Var[bool] = True
    is_searchable: rx.Var[bool] = True

    def get_event_triggers(self) -> dict[str, Any]:
        return {
            **super().get_event_triggers(),
            "on_change": lambda e0: [e0],
        }

class MultiSelectComponentState(rx.ComponentState):
    component_state_value: list[dict[str, str]] = []

    @classmethod
    def get_component(cls, *children, **props) -> rx.Component:
            on_change = props.pop("on_change", [])
            if not isinstance(on_change, list):
                on_change = [on_change]

            return MultiSelectComponent.create(
                *children,
                value=cls.component_state_value,
                on_change=[cls.set_component_state_value, *on_change],
                **props,
        )

multiselect = MultiSelectComponentState.create

# some_page.py  -- I.e. example usage
import reflex as rx
from .multi_select_component import multiselect

class MultiSelectState(rx.State):
    selected: List[dict[str, str]] = []

    @rx.cached_var
    def selected_values(self) -> str:
        return ", ".join([d["value"] for d in self.selected])

@rx.page(route="/multi_select", title="Multi Select")
def index() -> rx.Component:
    return rx.container(
        multiselect(
            options=[
                {"value": "opt1", "label": "Option 1"},
                {"value": "opt2", "label": "Option 2"},
            ],
            on_change=MultiSelectState.set_selected,
        ),
        rx.text(f"Multiselect value {MultiSelectState.selected_values}"),
    )

This seems to function as I would expect, so that is good. The only thing I am a bit suspicious of is that I now cannot set the value prop e.g. multiselect(..., value=MultitSelectState.selected, ...) because that value needs to be handled in the ComponentState.

Or maybe the answer is to add some initialization logic in the get_component method like this?

    @classmethod
    def get_component(cls, *children, **props) -> rx.Component:
            on_change = props.pop("on_change", [])
            if not isinstance(on_change, list):
                on_change = [on_change]

            value = props.get('value', None)
            if value is None:
                value = cls.component_state_value
                on_change = [cls.set_component_state_value, *on_change]
            else:
                if not on_change:
                    raise ValueError("MultiSelectComponent requires an on_change event handler if value is set.")

            return MultiSelectComponent.create(
                *children,
                value=value,
                on_change=on_change,
                **props,
        )

I.e., check that if a value is passed, there must also be an on_change event handler (and just hope that event handler will set the value passed in since that is required for the component to work).

As a bonus question... Is there any reasonably easy way to theme a custom component like this so that it will behave similarly to the rest of radix themed components? Or is it even possible at all? The theming system is pretty opaque to me at the moment.

masenf commented 7 months ago

It looks like you're on the right track. I think inspecting the props in get_component and making appropriate adjustments is the right move.

As for the theming, take a look at what we've done for progress: https://github.com/reflex-dev/reflex/blob/main/reflex/components/radix/primitives/progress.py

Some of the other components still need to be updated to use this mechanism, but what it boils down to is setting the style props using the radix theme tokens (var(--radius-2) and rx.color("accent", 9)) and then making sure the rendered component has the proper data-* attribute that influences what those CSS tokens render as.

For example, setting data_accent_color="red" and then using rx.color("accent", 9) will give you a red color suitable for active elements.

This is the way to tie into the radix themes system appropriately.

Currently we apply self.style changes in the _apply_theme method, but you can also apply these as the style prop in get_component.

TimChild commented 7 months ago

Thanks for the pointers! When I get a chance to come back to it I'll see if I can get it themed properly and try out publishing to pypi too.

abulvenz commented 5 months ago

@TimChild Maybe something like the app in this MR can also be of interest https://github.com/reflex-dev/reflex/pull/3375 Did you succeed in publishing to pypi, yet?

picklelo commented 5 months ago

Closing this issue as it's out of our scope - feel free to keep posting to track progress of the third party component.

TimChild commented 5 months ago

@abulvenz , Thanks for the suggestion! I've been too busy to come back to this yet, but it's still somewhere down there on my todo list...

I'll update if I do publish, but until then I agree this should be closed as out of scope for the reflex team.