google / mesop

Rapidly build AI apps in Python
https://google.github.io/mesop/
Apache License 2.0
5.65k stars 274 forks source link

Nested content_button can cancel event propagation #1119

Open jmfk opened 3 days ago

jmfk commented 3 days ago

Describe the bug If you have a button in a button there is no way to cancel the event from propagating to container event handler resulting in an error.

Suggested solution Make it possible to stop event propagation (e.g. event.stopPropagation())

Code To Reproduce

import mesop as me

projects = [ {"key": "hello", "text": "Hello", "tooltip": "This is a tooltip"}, {"key": "world", "text": "World", "tooltip": "This is another tooltip"}, ]

@me.stateclass class State: select_message: str = "Nothing yet" delete_message: str = "Nothing yet"

def on_reset(e: me.ClickEvent): global projects projects = [ {"key": "hello", "text": "Hello", "tooltip": "This is a tooltip"}, {"key": "world", "text": "World", "tooltip": "This is another tooltip"}, ]

def on_select_project(e: me.ClickEvent): state = me.state(State) state.select_message = f"Selected: {str(e)}"

def on_delete_project(e: me.ClickEvent): global projects state = me.state(State) state.delete_message = f"Deleted: {str(e)}" projects = [p for p in projects if p["key"] != str(e.key)]

@me.page( path="/", title="Tooltip Bug", ) def app(): state = me.state(State) with me.box(): me.text(f"select_message: {state.select_message}") me.text(f"delete_message: {state.delete_message}") me.button( on_click=on_reset, label="Reset", )

for project in projects:
    with me.box(
        style=me.Style(
            padding=me.Padding.symmetric(horizontal=10, vertical=8),
        )
    ):
        with me.content_button(
            on_click=on_select_project, key=project["key"], color="accent"
        ):
            with me.box(
                style=me.Style(
                    display="flex",
                    flex_direction="row",
                    justify_content="space-between",
                    align_items="center",
                    width="245px",
                    padding=me.Padding.symmetric(horizontal=6),
                )
            ):
                me.text(project["text"])
                with me.content_button(
                    on_click=on_delete_project,
                    type="icon",
                    key=project["key"],
                ):
                    me.icon(icon="delete_circle")
richard-to commented 3 days ago

I haven't tested though yet, but the me.click_event has an "is_target" parameter. It will not stop the propagation, but you can use it to check if it is the element the got clicked. However in this case it may not work for the outer content button (mainly due to the nested box).

I think the solution is to not use content button. Instead put the click event handler on the me.box? That may still work correctly depending on what happens if you click on the me.text.

So a modified version could look like:

           with me.box(
                on_click= on_select_project,
                style=me.Style(
                    display="flex",
                    flex_direction="row",
                    justify_content="space-between",
                    align_items="center",
                    width="245px",
                    padding=me.Padding.symmetric(horizontal=6),
                )
            ):  
                with me.box(on_click=on_select_project):  // May not be needed.
                  me.text(project["text"])
                with me.content_button(
                    on_click=on_delete_project,
                    type="icon",
                    key=project["key"],
                ):
                    me.icon(icon="delete_circle")

Then in your event handler, you'd have:

def on_select_project(e: me.ClickEvent):
  if e.is_target:
    state = me.state(State)
    state.select_message = f"Selected: {str(e)}"

I agree that this is more of hack/workaround to your issue. The is_target solution does not fully handle all the use cases that event propagation needs.

jmfk commented 3 days ago

I'll have a play with is_target, I didn't get any useful solution with it before but I'll check it again (still trying to figure things out here :-)).

Using me.box I lose the hover effect I get with me.content_button!? Unless there is way to implement custom hover effects on me.box? I would prefer to use it instead of nesting content_button, which seems a bit "wrong".

richard-to commented 2 days ago

I see. Yes, the inability to use hover using me.style is annoying.

As a workaround, for the hover effect, you can use custom CSS on the me.box(classes="your-class"). Then you can add your own custom stylesheet via local static folder (see https://google.github.io/mesop/guides/static-assets/)

I didn't get any useful solution with it before but I'll check it again (still trying to figure things out here :-)).

Yeah, we appreciate all the feedback. I think it's important stuff to know what people are trying to do with Mesop and it's important to know where it's falling short or not providing a user-friendly experience.