Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#15922 closed defect (fixed)

Cannot capture View's onStartView event

Reported by: ykami Owned by: Eric Durocher
Priority: undecided Milestone: 1.8.1
Component: DojoX Mobile Version: 1.8.0
Keywords: Cc: cjolif
Blocked By: Blocking:

Description

test_transition-connect.html is listening to the onStartView event of a View widget, but the event handler is never called.

(Reported by Martin Triska)

Attachments (1)

15922.patch (683 bytes) - added by Eric Durocher 7 years ago.
Put back event firing calls in a defer() call

Download all attachments as: .zip

Change History (14)

comment:1 Changed 7 years ago by ykami

In 1.7, there was setTimeout in View.startup(), but it was removed in 1.8. Firing onStartView should have been in the setTimeout.

comment:2 Changed 7 years ago by Eric Durocher

Cc: cjolif added

comment:3 Changed 7 years ago by cjolif

1/ do we really want to put that back in? If the user wants to set a callback on a not yet created object he needs to pass it to its constructor or in the markup? This sounds wrong to me to fire a setTimeout to let him actually register for a creation event *post* the creation itself?

2/ if we really want to keep that as is, shouldn't this use _WidgetBase.defer instead of setTimeout.

comment:4 Changed 7 years ago by Eric Durocher

1/ I agree that this was a little strange, but I would tend to preserve compatibility, with a comment to revisit this for 2.0.

2/ Agreed, although there is really little chance that the widget can get destroyed before the timeout fires, but it does save a few bytes of code so I buy it.

Changed 7 years ago by Eric Durocher

Attachment: 15922.patch added

Put back event firing calls in a defer() call

comment:5 Changed 7 years ago by ykami

cjolif, you'll see how it is used if you take a look at test_transition-connect.html and test_transition-pubsub.html. The purpose of startView is to allow the user to do something when the view becomes visible. Usually it can be done by using afterTransitionIn etc., but they are never fired at startup time, since no view transition is performed yet. That's why startView exists. So, setTimeout is actually necessary. Call of startView without setTimeout is too early since the view's initialization is not finished at that timing.
(Use of defer() is fine.)

comment:6 Changed 7 years ago by cjolif

ykami, I did see the test but as I said I think the handler should be passed into the ctor or markup which removes any need for setTimeout? If you really don't want to do that you can still listen the /dojox/mobile/startView topic that is dispatched globally and thus can be registered outside of the ctor (pre-creation of the object). I don't think using setTimeout to allow to attach event listeners *post* their dispatching is a good practice even if that indeed can simply the life of users in some cases (but also create confusion on how things work).

That said I can buy the compatibility argument from edurocher. We actually do have another problem reported about onStartView ideally we should try to see if putting that back in fixes both reported issues.

Last edited 7 years ago by cjolif (previous) (diff)

comment:7 Changed 7 years ago by ykami

ok, let me clarify a little more.

  • The purpose of startView is to provide the same API as afterTransitionIn etc. for a initially visible view for user's convenience. It is not convenient at all if the API is different.
  • Both passing a handler to the ctor and connecting to a view instance should work if the call of startView is delayed. If not, both do not work, because the timing is too early.
  • It is not allowing to attach event listeners *post* their dispatching. It is delaying the dispatching.

comment:8 Changed 7 years ago by cjolif

Ok sorry for my bad wording but by allowing to attach *post* dispatching I meant to attach *post* when the dispatching is *supposed* to happen. Which one more time, I think, might sound convenient but is also a source of confusion by creating dis-continuity in the event dispatching flow.

In your second point it seems you imply both cases do not work without setTimeout. About passing the handler to the ctor, I don't see why it would not work without setTimeout as it will be available for sure when the event will be dispatched? Or maybe you mean that the handler will be correctly called but too early? In that case I would be interested in understanding exactly what is the problem (in particular wouldn't dispatching it at the end of startup() instead of in the middle solve that second issue if it exists).

Last edited 7 years ago by cjolif (previous) (diff)

comment:9 Changed 7 years ago by cjolif

In [29618]:

refs #15922. Put back the dispatching of onStartView into a defer for compatibility reasons. Need to revisit this later. Thanks Eric Durocher (IBM, CCLA). !strict.

comment:10 Changed 7 years ago by cjolif

Milestone: tbd1.8.1

A unit test checking this is working (as this is pretty easy "to miss" the bad behavior in the functional test) would be welcome as well.

comment:11 Changed 7 years ago by cjolif

Resolution: fixed
Status: newclosed

In [29619]:

fixes #15922, #15878. Backport fixes from Eric Durocher (IBM, CCLA) into 1.8 branch. !strict.

comment:12 Changed 7 years ago by ykami

cjolif,

I don't see why it would not work without setTimeout

Without the delay, even though the user handler is called if you pass it to ctor, it is not useful because the view's initialization is not finished at that timing yet. That's what I meant. The intention was to provide something convenient that can be used the same way as afterTransitionIn etc. If you want to do it your way, there are many alternative ways to wait for view's initialization, and startView would not be necessary.

comment:13 Changed 7 years ago by cjolif

In [29677]:

refs #15922. Add a robot unit test for transition events.

Note: See TracTickets for help on using tickets.