Thiago,

the JavaDoc for PublishEvent is broken, please fix it.
https://travis-ci.org/apache/tapestry-5/builds/218065676#L570
https://travis-ci.org/apache/tapestry-5/jobs/217885298#L897

Jochen

Am 03.04.2017 um 15:01 schrieb Thiago H. de Paula Figueiredo:
On Mon, Apr 3, 2017 at 3:45 AM, Jochen Kemnade <jochen.kemn...@eddyson.de>
wrote:

Hi,


Hello!


Am 03.04.2017 um 00:35 schrieb Thiago H. de Paula Figueiredo:

I've investigated and I've found that the place I've added the getEventUrl
function in the Coffee file has caused some weird issues with the way the
JavaScript was output.


Yes, I'd fixed that already in [1].


Thanks! I was working on the 5.4.2 branch first, so I hadn't noticed it and
ended up fixing it in the branch then merged into master.


I'm really not a fan of Coffee.


We can always switch back to JS or use ES6 with Babel. This is what I do
for all of my JS now.


Hmm, I wouldn't consider changing the whole JS codebase from Coffee to
vanilla JS, ES6 or not, a good use of *my* very scarce time for Tapestry.
Of course, if someone wants to do that, I'd love it. :) For my personal
projects, I'd start using TypeScript from now on. ES6 + ES7 + type checking.



I'll probably create another 5.4.2 release/vote next week, if you'd like
to address some of the other issues before then.


+1!



Jochen

[1] https://git-wip-us.apache.org/repos/asf?p=tapestry-5.git;a=b
lobdiff;f=tapestry-core/src/main/preprocessed-coffeescript/
org/apache/tapestry5/t5-core-dom.coffee;h=97b2a0698d1da7194
11c43df4e6768665170fa89;hp=1f07d96e76997574b74fa57c3b88cc290
a9a78a4;hb=589ff43b17f80db1698b1c30257f808588a8f5c8;hpb=ea4d
b57b91fc8153a553f78c7a10b043080d4ce9



On Wed, Mar 29, 2017 at 3:02 AM, Jochen Kemnade <
jochen.kemn...@eddyson.de>
wrote:

Thiago,

do you think you'll find the time to address those issues in the near
future? I'd rather create a new 5.4.2 release than shipping the current
build with known issues.

Jochen

----- On Mar 24, 2017, at 1:25 PM, Thiago H. de Paula Figueiredo
thiag...@gmail.com wrote:

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!


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tapestry.apache.org
For additional commands, e-mail: dev-h...@tapestry.apache.org






---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tapestry.apache.org
For additional commands, e-mail: dev-h...@tapestry.apache.org






---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tapestry.apache.org
For additional commands, e-mail: dev-h...@tapestry.apache.org

Reply via email to