zauberzeug / nicegui

Create web-based user interfaces with Python. The nice way.
https://nicegui.io
MIT License
9.59k stars 567 forks source link

`ui.refreshable` with `ui.state` in an instance method #3392

Open kleynjan opened 2 months ago

kleynjan commented 2 months ago

Description

Below is an OO variant of the "counter" example given in the docs for ui.state. Only counter B gets working set_count and set_color functions. I've tried to move variables and setters to __init__ as self.counter, self.set_counter etc but that doesn't help.

from nicegui import ui

class Counter:
    def __init__(self, name: str):
        self.name = name

    @ui.refreshable_method     # or @ui.refreshable, same result
    def counter(self):
        with ui.card():
            count, set_count = ui.state(0)
            color, set_color = ui.state("black")
            ui.label(f"{self.name} = {count}").classes(f"text-{color}")
            ui.button(f"{self.name} += 1", on_click=lambda: set_count(count + 1))
            ui.select(["black", "red", "green", "blue"], value=color, on_change=lambda e: set_color(e.value))

with ui.row():
     Counter("A").counter()
     Counter("B").counter()

ui.run()

Looking at ui.state source in refreshable.py, the state variables and setters functions are added to the local scope of the current target, but that mechanism seems to be broken when the @refreshable is a method?

This is on 1.4.29.

python-and-fiction commented 2 months ago

It works well after modify.

from nicegui import ui

class Counter:
    def __init__(self, name: str):
        self.name = name
    def counter(self):
        @ui.refreshable_method    # or @ui.refreshable, same result
        def counter():
            with ui.card():
                count, set_count = ui.state(0)
                color, set_color = ui.state("black")
                ui.label(f"{self.name} = {count}").classes(f"text-{color}")
                ui.button(f"{self.name} += 1", on_click=lambda: set_count(count + 1))
                ui.select(["black", "red", "green", "blue"], value=color, on_change=lambda e: set_color(e.value))
        return counter()

with ui.row():
     Counter("A").counter()
     Counter("B").counter()

ui.run()
kleynjan commented 2 months ago

Thanks! I understand we can make it work with a closure like this, but it feels like a kludge. The lifecycle for "refreshables" is mysterious enough as it is. 🙂

python-and-fiction commented 2 months ago

Another example,maybe you needn't create a new instance but call it twice.

from nicegui import ui

class Counter:
    def __init__(self, name: str):
        self.name = name
    @ui.refreshable    # or @ui.refreshable, same result
    def counter(self,name=None):
        with ui.card():
            count, set_count = ui.state(0)
            color, set_color = ui.state("black")
            ui.label(f"{self.name if name is None else name} = {count}").classes(f"text-{color}")
            ui.button(f"{self.name if name is None else name} += 1", on_click=lambda: set_count(count + 1))
            ui.select(["black", "red", "green", "blue"], value=color, on_change=lambda e: set_color(e.value))

with ui.row():
    a = Counter("A")
    a.counter()
    a.counter('B')

ui.run()

image

kleynjan commented 2 months ago

@python-and-fiction thanks for the workarounds. For now I've concluded that ui.state doesn't fit my use case. I'll come back to that later, in discussions.

@falkoschindler , @rodja et al, I flagged the issue because I think it's a bug or an anomaly. If it's an edge case that can be ignored, please close the issue.

falkoschindler commented 2 months ago

That's an interesting problem, @kleynjan! I think we should leave it open as a bug.

I just experimented with your code

class Counter:
    def __init__(self, name: str) -> None:
        self.name = name

    @ui.refreshable_method
    def counter(self) -> None:
        with ui.card():
            count, set_count = ui.state(0)
            ui.label(f'{self.name} = {count}')
            ui.button(f'{self.name} += 1', on_click=lambda: set_count(count + 1))

Counter('A').counter()
Counter('B').counter()

and it looks like some target or target.refreshable points to a wrong instance. But I'll have to dive a bit deeper to wrap my head around it, because some things are kind of static and always point to the same instance and others aren't. It's confusing.

I also compared it to the non-OOP variant

@ui.refreshable
def counter(name: str) -> None:
    with ui.card():
        count, set_count = ui.state(0)
        ui.label(f'{name} = {count}')
        ui.button(f'{name} += 1', on_click=lambda: set_count(count + 1))

with ui.row():
    counter('C')
    counter('D')

which works in principal, but seems to update both cards C and D when incrementing the counter. In case you have many counters on a page, this causes quite some overhead. Maybe that's related to the OOP problem.