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

[REF-554] Wrapping event triggers of React components with more than one parameter #1762

Closed
janhavlin opened this issue Sep 6, 2023 · 2 comments · Fixed by #1820
Closed
Labels
feature request A feature you wanted added to reflex linear Created by Linear-GitHub Sync Medium priority Created by Linear-GitHub Sync

Comments

@janhavlin
Copy link

janhavlin commented Sep 6, 2023

Hello, I'm struggling with wrapping callbacks of FormSelect component from @patternfly/react-core library. The issue is that the onChange callback has two parameters ((event: React.FormEvent<HTMLSelectElement>, value: string) => void, see the docs) but I'm able to generate only one parameter in the callback in the resulting javascript code.

I have the following code:

import reflex as rx

class FormSelect(rx.Component):
    library = "@patternfly/react-core"
    tag = "FormSelect"
    value: rx.Var[str]

    @classmethod
    def get_controlled_triggers(cls) -> dict[str, rx.Var]:
        return {
            "on_change": rx.EVENT_ARG
        }

class FormSelectOption(rx.Component):
    library = "@patternfly/react-core"
    tag = "FormSelectOption"
    value: rx.Var[str]
    label: rx.Var[str]

form_select = FormSelect.create
form_select_option = FormSelectOption.create

class State(rx.State):
    selected_value: str
    def process_change(self, value):
        self.selected_value = value

def index() -> rx.Component:
    return rx.fragment(
        form_select(
            form_select_option(label='value1', value='value1'),
            form_select_option(label='value2', value='value2'),
            value=State.selected_value,
            on_change=State.process_change
        ),
    )

app = rx.App(state=State)
app.add_page(index)
app.compile()

Which results in this javascript which is not correct, onChange callback should have more than just the _e argument:

  <Fragment>
  <FormSelect onChange={_e => Event([E("state.process_change", {value:_e})], _e)} value={state.selected_value}>
  <FormSelectOption label="value1" value="value1"/>
  <FormSelectOption label="value2" value="value2"/>
</FormSelect>
</Fragment>

I was able to "fix" it with manual touches to the javascript code:

  <FormSelect onChange={(_e, v) => Event([E("state.process_change", {value: v})], _e)} value={state.selected_value}>

But I don't know how to achieve this within the python code. I tried adding an extra parameter to the python handler:

class State(rx.State):
    selected_value: str
    def process_change(self, event, value):
        self.selected_value = value

But no luck:

AssertionError: Event handler <function State.process_change at 0x7fac7bb83420> must have 1 or 2 arguments.

Thanks in advance for any help!

REF-554

@masenf masenf added the linear Created by Linear-GitHub Sync label Sep 6, 2023
@masenf
Copy link
Collaborator

masenf commented Sep 6, 2023

Unfortunately this is not supported in the current reflex version... The EVENT_ARG thing is relatively hardcoded throughout the python -> javascript translation process, but I think we should be more flexible here. It's not going to be an easy fix as far as I can tell; some refactoring will be needed to support callbacks with arbitrary number of arguments.

@masenf masenf added the feature request A feature you wanted added to reflex label Sep 6, 2023
@masenf masenf added linear Created by Linear-GitHub Sync and removed linear Created by Linear-GitHub Sync labels Sep 7, 2023
@masenf masenf changed the title Wrapping event triggers of React components with more than one parameter [REF-554] Wrapping event triggers of React components with more than one parameter Sep 7, 2023
@Lendemor
Copy link
Collaborator

Will be solved by #1820

@Lendemor Lendemor linked a pull request Sep 16, 2023 that will close this issue
7 tasks
@masenf masenf added the Medium priority Created by Linear-GitHub Sync label Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request A feature you wanted added to reflex linear Created by Linear-GitHub Sync Medium priority Created by Linear-GitHub Sync
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants