GenieFramework / Stipple.jl

The reactive UI library for interactive data applications with pure Julia.
MIT License
323 stars 27 forks source link

`@click` handler doesn't work `v-on:click` works #164

Open AbhimanyuAryan opened 1 year ago

hhaensel commented 1 year ago

Are you aware that there are two methods for @click?

julia> @click(:me)
"v-on:click='me = true'"

julia> @click("console.log('Hi')")
"v-on:click='console.log(\\'Hi\\')'"
hhaensel commented 1 year ago

It's all in the docstring... Otherwise, please send a MWE.

AbhimanyuAryan commented 1 year ago

@hhaensel helmut I can vaguely recall seeing that first signature somewhere with symbol. I always fallback to using second one @click("bla = true"). It's better to say I don't know the first signature. I have recently started paying more attention to Stipple because I want to add few features that I like in other frameworks and are really close to my heart. So will be asking you some doubts in discord comings weeks

AbhimanyuAryan commented 1 year ago

@hhaensel regarding the bug. I need check it again because I came across it first time when I was building a GenieBuilder app with Dani. I have not faced this in Stipple specifically. I'll create MWE and post. Thanks :)

I took a note of this here because it was a few months back and I lost track of it. So didn't want to miss it again because jeremy reported this too

AbhimanyuAryan commented 1 year ago

just looked at the definition of macro click in StippleUI. Will look into this issue myself. But thanks a lot @hhaensel especially the last issue I created. You have contributed a lot on that and I going thought it

jeremiedb commented 1 year ago

@hhaensel A motivation for having the @click to apply the vue logic was to be able to simply copy-paste code patterns as found in the Quasar docs.

For examle, in a Stipple UI demo I'm preparing, I ended up using: <q-btn v-on:click="button_reset = true" color="secondary" label="Secondary" /> https://github.com/jeremiedb/GenieExperiments.jl/blob/7d3735811acbece8ffa08a3ae5a6039b304c6d3a/app/resources/UIDemo/views/ui.jl.html#L85

while I think it would be desirable to be able to directly use Quasar snippets such as <q-btn :loading="progress" color="primary" @click="progress = true"> as found in https://v1.quasar.dev/vue-components/button#progress-related

hhaensel commented 1 year ago

Quasar Snippets

When taking snippets from the internet, it's probably a good idea to make them ParsedHTMLStrings and not just Strings. Theoretically they shouldn't be parsed and the @-sign shouldn't make problems. Infortunately, it still does, and I don't know yet why.

(@essenciary , we should find out why the @-sign is not correctly transmitted.)

Good news

There is help. With the latest release of StippleUI we have released StippleUIParser, which can take snippets from the internet and turn them into julia code. It's not yet perfect but a very good starting point, I'd say.

julia> doc_string = """
<template>
    <div class="q-pa-md">
    <q-scroll-area style="height: 230px; max-width: 300px;">
        <div class="row no-wrap">
            <div v-for="n in 10" :key="n" style="width: 150px" class="q-pa-sm">
                Lorem @ipsum dolor sit amet consectetur adipisicing elit. Architecto fuga quae veritatis blanditiis sequi id expedita amet esse aspernatur! Iure, doloribus!
            </div>
            <q-btn color=\"primary\" label=\"`Animate to \${position}px`\" @click=\"scroll = true\"></q-btn>
            <q-input hint=\"Please enter some words\" v-on:keyup.enter=\"process = true\" label=\"Input\" v-model=\"input\" class=\"q-my-md\"></q-input>
            <q-input hint=\"Please enter a number\" label=\"Input\" v-model.number=\"numberinput\" class=\"q-my-md\"></q-input>
        </div>
    </q-scroll-area>
    </div>
</template>
""";

julia> parse_vue_html(doc_string) |> println
template(
    Stipple.Html.div(class = "q-pa-md", 
        scrollarea(style = "height: 230px; max-width: 300px;", 
            Stipple.Html.div(class = "row no-wrap", [
                Stipple.Html.div(var"v-for" = "n in 10", key! = "n", style = "width: 150px", class = "q-pa-sm", 
                    Lorem @ipsum dolor sit amet consectetur adipisicing elit. Architecto fuga quae veritatis blanditiis sequi id expedita amet esse aspernatur! Iure, doloribus!
                )
                btn("`Animate to \${position}px`", color = "primary", var"v-on:click" = "scroll = true")
                textfield("Input", :input, hint = "Please enter some words", var"v-on:keyup.enter" = "process = true", class = "q-my-md")
                numberfield("Input", :numberinput, hint = "Please enter a number", class = "q-my-md")
            ])
        )
    )
)

Note: Currently the quotes around Lorem ipsum ... are not yet rendered, but that will be fixed in the next version. VS Code will tell you what's wrong :wink:

hhaensel commented 1 year ago

@hhaensel A motivation for having the @click to apply the vue logic was to be able to simply copy-paste code patterns as found in the Quasar docs. ... while I think it would be desirable to be able to directly use Quasar snippets such as <q-btn :loading="progress" color="primary" @click="progress = true"> as found in https://v1.quasar.dev/vue-components/button#progress-related

@jeremiedb Most probably, you can use the code snippets, if you wrap them in ParsedHTMLString, so

ParsedHTMLString("""<q-btn :loading="progress" color="primary" @click="progress = true">""")

as part of the ui() function should work, as it will not be parsed.

Alternatively, you can use the StippleUIParser, which I introduced in my previous post. Your code snippet would then be converted like this

julia> parse_vue_html("""<q-btn :loading="progress" color="primary" @click="progress = true">""") |> println
btn(loading = :progress, color = "primary", var"v-on:click" = "progress = true")

The result can be directly pasted in your code and modified according to your needs. Here's the validity check:

julia> btn(loading = :progress, color = "primary", var"v-on:click" = "progress = true")
"<q-btn color=\"primary\" :loading=\"progress\" label v-on:click=\"progress = true\"></q-btn>"
hhaensel commented 1 year ago

@jeremiedb , @AbhimanyuAryan can this be closed?

PGimenez commented 1 year ago

@hhaensel I came across this while trying to get @row-clickworking for tables, and your suggested workaround with ParsedHTMLString is not working for me. Am I missing something? Here's a MWE:

using GenieFramework
@genietools

@app begin
    @in i = 0
    @onchange i begin
        @show i
    end
end

function ui()
    [
        p("{{i}}")
        btn("Change", @click("i = i+1"))
        ParsedHTMLString("""<q-btn @click="i = i+1">Change @click</q-btn>""")
        """<q-btn v-on:click="i = i+1">Change v-on</q-btn>"""
    ]
end

@page("/", ui)
up()
hhaensel commented 11 months ago

With the latest fixes, you're almost there. Just make sure that all elements of the vector are ParsedHTMLStrings. You could just pipe the vector to ParsedHTMLString

function ui()
           [
               p("{{i}}")
               btn("Change", @click("i = i+1"))
               """<q-btn @click="i = i+1">Change @click</q-btn>"""
               """<q-btn v-on:click="i = i+1">Change v-on</q-btn>"""
           ] |> ParsedHTMLString
       end
hhaensel commented 11 months ago

So finally we got @page working for unregistered components.

@essenciary Currently you throw an error when you find an unregistered element during parsing because you assume that you can't build it with Julia code, but you could, indeed, build it with the help of xelem() from StippleUI.API (or a copy of that function). What do you think?

EDIT: Should I open an issue for that?

essenciary commented 11 months ago

The error is thrown by design so the users can register the component. There was a system that automatically registered components but this just hides potential errors (like a typo).

hhaensel commented 11 months ago

What is wrong about using unregistered components?

hhaensel commented 11 months ago

Currently you cannot use unregistered components together with sophisticated Julia code templates, because the latter is only available during parsing.

hhaensel commented 11 months ago

Couldn't you offer a switch for accepting unregistered components or at least throw an error that tells you the code to register a component?

essenciary commented 11 months ago

What is wrong about using unregistered components?

A few things.

The issue is caused by the fact that HTML elements are represented as Julia functions. If a component is not registered, it means the corresponding function does not exist. Naturally, this throws an exception.

My initial solution was to capture the exception and automatically register a function. But on second thought, the issues were:

1/ throwing and catching exceptions is quite expensive in terms of performance. If we allow this, people will just use it, and the performance will be badly impacted. 2/ it would allow errors like a typo in an element to not be caught, resulting in very hard to fix bugs (like say <spam> instead of <span>).

The switch would cause the same issues - unless we come up with an implementation that does not rely on catching Julia errors, add a warning that an unregistered component was registered (which could be missed), etc.

So on this one I'd err on the side of explicitness. Sometimes making things too easy makes it easy to shoot yourself in the foot.

Improving the error message with the hint on how to solve the error is a good idea.

hhaensel commented 11 months ago

There is an easy and fast method to detect whether a julia function exists, just call methods() on it and evaluate the result.

I did this in an even more advanced way for parse_vue_html(), because I also detected the arguments, with which the function has to be called.

Another approach would be to simply call xelem(tag, ...) or even a simpler version of it without lots of attribute conversion. No need to register any function. Moreover, I would add a switch to replace @ characters, which I did in my version. (It's all in StippleUI. We discussed about that, do you remember?)

essenciary commented 11 months ago

I don't see these as being better.

Also, I don't see the issue. In JS, if you want to use a component, you register it. If you want to (also) use it in Julia, register it. Why is this problematic? Registering the component in Julia should also ideally be managed via a package, JS should be injected dynamically, etc. These are just overall better practices vs adding some random JS and introducing performance issues in Julia to make them work while avoiding one line of Julia code to register it. Maybe I'm missing something.

essenciary commented 11 months ago

Upon further consideration, with register_element a dedicated rendering method is added and is then available for all the duration of the app's execution, eliminating the error. While with xelem the error persists on every parsing, making for worse performance.

essenciary commented 11 months ago

To summarize:

The (optional) auto registration of the component/HTML element is the best approach, but really a hack. It would potentially severely impact the performance of the first rendering, which will be worst felt in production, coupled with TTFx. In contrast, it can easily be fixed in development, by registering the component/element explicitly -- with a better error message that explains the solution.

hhaensel commented 11 months ago

Upon further consideration, with register_element a dedicated rendering method is added and is then available for all the duration of the app's execution, eliminating the error. While with xelem the error persists on every parsing, making for worse performance.

I disagre, xelem works without registering and could be used for all elements. In conrtast, I find it very strange to register the identical function with just the tag changed for each element.

If you like I could come up with some demo code that is ported from parse_vue_html.

essenciary commented 11 months ago

It's not strange, because then you have code generating functions, and you can write low-code like this:

div(...) do 
    p("Hello") 
    span("Today is ...")
    badge(color=red)
end

Without it, you'll just have

xelem(div, [
  xelem(p, "...")
  xelem(span, "...")
  xelem(badge, "...")
])

Right?

hhaensel commented 11 months ago

Right, but what's bad about this? The two should have identical performance and the latter doesn't need precompilation for every element type.

essenciary commented 11 months ago

Assuming that we're still both talking about handling of unregistered components, it's about the number of operations. Say you develop your app, writing your HTML UI. The HTML is parsed 300 times in a coding session as you keep changing it and reloading.

Flow per parsing:

xelem: a) component rendering function is not found b) error is thrown c) xelem is called d) component is rendered

-> repeat a)-d) every time the HTML is parsed

register_element: a) component rendering function is not found b) error is thrown c) component rendering function is registered d) component is rendered

-> when html is parsed next time, skip a) -> c) and go directly to d)

The xelem approach runs in O(n) (repeats every time a missing component is found) while the register_element approach runs in constant time (only happens once per component).

https://en.wikipedia.org/wiki/Big_O_notation

hhaensel commented 11 months ago

Maybe I'm missing something. I would not try to use a registered function at all, so there won't be any a) or b), just call xelem for each of the elements.

essenciary commented 11 months ago

Probably we're talking about different things then...

essenciary commented 11 months ago

My details followed these topics, if I understood your questions correctly:

1/ you - why do unregistered components throw errors? me - Unregistered components result as missing Julia functions in the HTML parser.

2/ you - why are there functions? me - because Genie defines a low-code API that maps HTML components to Julia functions allowing you to write HTML in Julia

3/ you - why don't we auto-register the functions? me - because it bad for performance and easy to solve by the user

4/ you - why don't we use xelem instead of registering functions? me - because of it's increased time complexity

5/ you - but why register functions? me - see question 2

essenciary commented 11 months ago

Adding these details as well:

a) Genie provides a low-code API to generate HTML - the p(...), span(...), slider(...) etc API. With this one can create HTML UIs in Julia b) Genie also provides a HTML based templating engine which allows you to use HTML directly - with additional dynamic features like embedding variables, having partials and layouts, loops and logical constructs, etc. c) parsing a large HTML template to apply the dynamic logic is slow - so Genie implements a build system where a piece of HTML is parsed and a Julia rendering function, using the low-code API at point a), is generated.

So these are the 2 sides of the coin: I) low-code API => HTML output II) HTML template => low-code API (=> HTML output)

essenciary commented 11 months ago

One way to solve it without too much hassle is to add a configuration option to auto-register components but have it false by default. Then people who know what they're doing, can set it to true if they want to. We can also indicate these in the error message.

hhaensel commented 11 months ago

There's, hopefully, only one point that I did not understand; why can the low-code API not use xelem only. In my html parser I look for matching methods based on the method signature (which is probably slow but not as slow as throwing an error), if I don't find a match I use xelem(), which should always work. But I could use xelem right away. If my goal is only to have a code-generating function I don't care how the code looks like. I don't need to see p or div. Just by looking at it, it seems to me that your construction with do blocks might be rather slow, because each do-block is an anonymous function which needs to be compiled first. This is typically slow in Julia other than in javascript. So why not using a function that simply takes the arguments of an array. Shouldn't that be faster? I guess that your solution is a bit slower in buidling and same speed as mine in providing the result. But I would build a version of xelem without big attribute handling. (I hope we are still on the same page ...)

essenciary commented 11 months ago

You don't need to use do blocks, you can also use arrays with the low-code.

If my goal is only to have a code-generating function I don't care how the code looks like.

That is not the only goal. There are 2 features around this. One is allowing users to write HTML UIs with Julia via the low-code API. They directly use the low-code API to write their user interfaces. The low-code API is for them and we do care what the code looks like.

For the 2nd feature, the template engine and building, yes, the auto-built code could use xelem instead of the low-code API.

The arguments to not using xelem are:

a) the auto-generated code can be user facing - when you have a code error in the template it will error out in the built rendering function, requiring the user to open it and debug it. Looking at a page of Julia code that only has hundreds of xelem calls is hard to read and debug.

b) no real benefit for the change. The low-code API is already available and generated at compile time and it's ready to use. What is the benefit to use xelem? Switch everything to xelem so users don't need to register elements? I already offered a 5 minutes implementation to address that (a setting).

hhaensel commented 10 months ago

So your offering is a switch to autoregister in case of unregistered functions, right?

I'd be fine with it. The only thing that you mentioned is that registering takes time. I thought that one way of circumventing that would be to use xelem for unregistered functions.

But again, I#m fine with the autoregister approach as well.