Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
AbhimanyuAryan opened this issue Jan 18, 2023 · 33 comments
Open

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

AbhimanyuAryan opened this issue Jan 18, 2023 · 33 comments
Assignees

Comments

@AbhimanyuAryan
Copy link
Member

No description provided.

@hhaensel
Copy link
Member

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
Copy link
Member

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

@AbhimanyuAryan
Copy link
Member Author

@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
Copy link
Member Author

@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
Copy link
Member Author

AbhimanyuAryan commented Jan 19, 2023

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
Copy link
Contributor

@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
Copy link
Member

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 😉

@hhaensel
Copy link
Member

@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
Copy link
Member

@jeremiedb , @AbhimanyuAryan
can this be closed?

@PGimenez
Copy link
Member

@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
Copy link
Member

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
Copy link
Member

hhaensel commented Nov 10, 2023

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
Copy link
Member

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
Copy link
Member

What is wrong about using unregistered components?

@hhaensel
Copy link
Member

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

@hhaensel
Copy link
Member

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
Copy link
Member

essenciary commented Nov 10, 2023

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
Copy link
Member

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
Copy link
Member

essenciary commented Nov 13, 2023

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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

@essenciary
Copy link
Member

essenciary commented Nov 15, 2023

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

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
Copy link
Member

hhaensel commented Dec 8, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants