widgetti / ipyreact

React for ipywidgets that just works. No webpack, no npm, no hassle
BSD 3-Clause "New" or "Revised" License
104 stars 8 forks source link

Clarification and Naming Convention for Prop Values and Setters in React Components on the Python Side #26

Closed kolibril13 closed 1 year ago

kolibril13 commented 1 year ago

There was still an open conversation in https://github.com/widgetti/ipyreact/pull/11 regarding naming with @paddymul and @maartenbreddels. I'm converting this to an issue, so it does not get lost. The title of this issue was generated by ChatGPT.

image
maartenbreddels commented 1 year ago

Did you ask gpt's opinion on on vs set ? :)

kolibril13 commented 1 year ago

Just asked, with the prev. conversation in context, and here is the answer:

image
maartenbreddels commented 1 year ago

What about "does on give the impression that it is an event handler, vs set which might be a better name for a setter. Give pros and cons then give your advice" Is this 3.5 or 4?

kolibril13 commented 1 year ago

that's the free version gpt3.5.

image
maartenbreddels commented 1 year ago

I can see from the quality of the answers it is not 4 :) They don't really make sense, but I think set_ should be the way, what do you think?

kolibril13 commented 1 year ago

Looking at these two examples, I think set_ is more intuitive.

class MyCounterWidget(ipyreact.ReactWidget):
    _esm = """
    import * as React from "react";

    export default function MyButton({value, on_value}) {
        return <button onClick={() => on_value(value + 1)}>
            {value || 0} times clicked
        </button>
    };"""
m = MyCounterWidget()
class MyCounterWidget(ipyreact.ReactWidget):
    _esm = """
    import * as React from "react";

    export default function MyButton({value, set_value}) {
        return <button onClick={() => set_value(value + 1)}>
            {value || 0} times clicked
        </button>
    };"""
m = MyCounterWidget()
m

but then looking at these two examples, I think on_ is more intuitive:

import ipyreact
from traitlets import Int, Unicode

class Widget1(ipyreact.ReactWidget):
    my_count = Int(0).tag(sync=True)
    label = Unicode("Click me").tag(sync=True)

    def on_my_python_function(self):
        self.my_count = self.my_count + 1
        self.label = f"Clicked {self.my_count}"

    _esm = """
        import * as React from "react";

        export default function({on_my_python_function, label}) {
            return(
            <div>
                <button onClick={() => on_my_python_function()}>
                    {label}
                </button>
            </div>
            )
        };
    """

w1 = Widget1()
w1
import ipyreact
from traitlets import Int, Unicode

class Widget1(ipyreact.ReactWidget):
    my_count = Int(0).tag(sync=True)
    label = Unicode("Click me").tag(sync=True)

    def set_my_python_function(self):
        self.my_count = self.my_count + 1
        self.label = f"Clicked {self.my_count}"

    _esm = """
        import * as React from "react";

        export default function({set_my_python_function, label}) {
            return(
            <div>
                <button onClick={() => set_my_python_function()}>
                    {label}
                </button>
            </div>
            )
        };
    """

w1 = Widget1()
w1
maartenbreddels commented 1 year ago

But they are not the same, one is setting a trait (and set should be used there), the otherone calls a python 'event' function, so there `onmakes sense. So I suggest we only change thevalue, on_valuepairs tovalue, set_value, and keep theon_some_event` names the same.

kolibril13 commented 1 year ago

Nice, I think, then set_value and on_some_event is a great combination!

kolibril13 commented 1 year ago

Closed by https://github.com/widgetti/ipyreact/pull/27