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

Get action, cancel, on_dismiss and on_auto_close working for rx.toast #3216

Merged
merged 8 commits into from May 15, 2024

Conversation

masenf
Copy link
Collaborator

@masenf masenf commented May 2, 2024

Handle callbacks into reflex event handlers.

Respect defaults set in ToastProvider toast_options when firing a toast with it's own ToastProps set.

Create new reflex.utils.format.format_queue_events helper method that can be used to take arbitrary eventhandler/eventspec/lambda along with an args spec, and create a callback function for use with rx.call_script-based APIs.

@masenf masenf mentioned this pull request May 2, 2024
Lendemor
Lendemor previously approved these changes May 2, 2024
reflex/components/sonner/toast.py Outdated Show resolved Hide resolved
specs = [spec]
events.extend(format.format_event(s) for s in specs)
on_click = Var.create(
f"() => {{queueEvents([{','.join(events)}], socket); processEvent(socket)}}",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this could be exposed as a formatting method that user that wrap component can use?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to rx.utils.format.format_queue_events

i think this will be really useful for any rx.call_script-based API that needs to pass callback functions that trigger reflex events, so good to formalize it.

and since i formalized it, it was also easy to add in on_dismiss and on_auto_close callbacks too 😎

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh nice, i realized i was also using a variant of this code inside call_script itself, so i can clean that up too 😄

_var_is_local=False,
)
else:
on_click = Var.create("() => null", _var_is_string=False, _var_is_local=False)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can pass this kind of var here, it would probably be possible to allow it for built-in event triggers too right ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Little known feature; we actually do allow it if you set the _var_type to EventChain, it just gets rendered as is

elif isinstance(value, EventChain):
# Trust that the caller knows what they're doing passing an EventChain directly
return value

if event_chain._var_type is not EventChain:
raise ValueError(f"Invalid event chain: {event_chain}")
return "".join(
[
"(() => {",
format_var(event_chain),
f"; preventDefault({format_var(event_arg)})" if event_arg else "",
"})()",
]
)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have to remember that one 👍

@masenf
Copy link
Collaborator Author

masenf commented May 3, 2024

@Lendemor idk if you want to take this into your branch (it has a refresh from origin/main); or if you just want to take your PR into main and then i can rebase this one and bring it into main separately.

@Lendemor
Copy link
Collaborator

Lendemor commented May 3, 2024

@Lendemor idk if you want to take this into your branch (it has a refresh from origin/main); or if you just want to take your PR into main and then i can rebase this one and bring it into main separately.

I'm fine with both, let's just go with whatever is more convenient.

masenf and others added 3 commits May 3, 2024 12:28
Respect defaults set in ToastProvider toast_options when firing a toast with
it's own ToastProps set.
Implement on_auto_close and on_dismiss callbacks inside ToastProps
Replace duplicate logic in rx.call_script for handling the callback function.
@masenf masenf changed the title Get action and cancel working for rx.toast Get action, cancel, on_dismiss and on_auto_close working for rx.toast May 3, 2024
@masenf masenf changed the base branch from lendemor/add_toast_component to main May 3, 2024 19:42
@masenf masenf dismissed Lendemor’s stale review May 3, 2024 19:42

The base branch was changed.

@masenf masenf closed this May 3, 2024
@masenf masenf reopened this May 3, 2024
"onClick": format.format_queue_events(action.on_click),
}


class PropsBase(Base):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think PropsBase should be exposed under _x too.
I've been playing with some libs that make extensive use of hooks, and the feature from this would be needed for them too.

Maybe in its own file since the code for it is not specific to sonner/toast

@Lendemor Lendemor mentioned this pull request May 14, 2024
This base class will be exposed via rx._x.PropsBase and can be shared by other
wrapped components that need to pass a JS object full of extra props.
@Lendemor Lendemor merged commit 76c8b2d into main May 15, 2024
46 checks passed
benedikt-bartscher pushed a commit to benedikt-bartscher/reflex that referenced this pull request May 16, 2024
…rx.toast (reflex-dev#3216)

* Get `action` and `cancel` working for rx.toast

Respect defaults set in ToastProvider toast_options when firing a toast with
it's own ToastProps set.

* Update reflex/components/sonner/toast.py

Co-authored-by: Thomas Brandého <[email protected]>

* Move queueEvent formatting into rx.utils.format module

Implement on_auto_close and on_dismiss callbacks inside ToastProps

* Update rx.call_script to use new format.format_queue_events

Replace duplicate logic in rx.call_script for handling the callback function.

* Move PropsBase to reflex.components.props

This base class will be exposed via rx._x.PropsBase and can be shared by other
wrapped components that need to pass a JS object full of extra props.

---------

Co-authored-by: Thomas Brandého <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants