On Fri, Mar 24, 2017 at 5:03 AM, Jochen Kemnade <jochen.kemn...@eddyson.de>
wrote:

> Thiago,
>

Hello!


> I've had a look at the new code and here's what I've found so far:
>
> The @PublishEvent JavaDoc is misleading:
> It says that the event handler method is "to be called in JavaScript
> through the t5/core/triggerServerEvent function", but there is no such
> function.
>

Oh, the annotation was the first thing I've wrote for this ticket and my
idea of what the JS API at the time for this would be. I'll fix this.


> An ElementWrapper (e.g. dom.body or dom('#foo')) passed as `element` is
> not handled correctly by getEventUrl. You should probably add
> if element instanceof ElementWrapper
>   element = element.element
>

Good catch! Thanks!


> Shouldn't dom.getEventUrl throw or at least warn if it cannot determine
> the url for the event, e.g. if you forget to add the @PublishEvent
> annotation?
>

Right now it returns null. Well, another good idea. :) Or maybe we could
leave the say it is and let the caller handle null values and properly
document this

The code I've wrote so far doesn't have automated tests, but the demo page
is actually a test: notice the Expected and the Value columns. They should
match for each row.

Did you create 5.4.2 with or without this feature?

Thank you very much!

Reply via email to