Stipple.jl icon indicating copy to clipboard operation
Stipple.jl copied to clipboard

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

Open AbhimanyuAryan opened this issue 2 years ago • 33 comments

AbhimanyuAryan avatar Jan 18 '23 13:01 AbhimanyuAryan

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 avatar Jan 18 '23 20:01 hhaensel

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

hhaensel avatar Jan 18 '23 20:01 hhaensel

@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 avatar Jan 19 '23 05:01 AbhimanyuAryan

@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 avatar Jan 19 '23 05:01 AbhimanyuAryan

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

AbhimanyuAryan avatar Jan 19 '23 08:01 AbhimanyuAryan

@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

jeremiedb avatar Jan 19 '23 20:01 jeremiedb

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 avatar Mar 20 '23 23:03 hhaensel

@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 avatar Mar 26 '23 21:03 hhaensel

@jeremiedb , @AbhimanyuAryan can this be closed?

hhaensel avatar Apr 17 '23 21:04 hhaensel

@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()

PGimenez avatar Sep 22 '23 10:09 PGimenez

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 avatar Nov 10 '23 10:11 hhaensel

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?

hhaensel avatar Nov 10 '23 10:11 hhaensel

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).

essenciary avatar Nov 10 '23 10:11 essenciary

What is wrong about using unregistered components?

hhaensel avatar Nov 10 '23 11:11 hhaensel

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

hhaensel avatar Nov 10 '23 11:11 hhaensel

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?

hhaensel avatar Nov 10 '23 11:11 hhaensel

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.

essenciary avatar Nov 10 '23 12:11 essenciary

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?)

hhaensel avatar Nov 13 '23 08:11 hhaensel

I don't see these as being better.

  • calling xelem instead of register_element may be "cheaper", but creating and throwing exceptions is expensive and using exceptions as a "feature" is not good practice.
  • avoiding exceptions and checking the existence of every function call, for a real-life sized DOM, will add a huge volume of computation, so not a go. Keep in mind that Genie templates are used for generating potentially huge static HTML web pages.

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 avatar Nov 13 '23 18:11 essenciary

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 avatar Nov 13 '23 21:11 essenciary

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.

essenciary avatar Nov 13 '23 21:11 essenciary

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.

hhaensel avatar Nov 15 '23 09:11 hhaensel

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?

essenciary avatar Nov 15 '23 11:11 essenciary

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

hhaensel avatar Nov 15 '23 14:11 hhaensel

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

essenciary avatar Nov 15 '23 14:11 essenciary

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.

hhaensel avatar Nov 15 '23 17:11 hhaensel

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

essenciary avatar Nov 15 '23 18:11 essenciary

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 avatar Nov 15 '23 19:11 essenciary

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 avatar Nov 15 '23 19:11 essenciary

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.

essenciary avatar Nov 15 '23 20:11 essenciary