[Laszlo-reviews] For Review: Change 20100623-maxcarlson-H Summary: Automatically turn on scrollevents when dependent events are registered

[P T Withington]

I like the spirit of this, but have some reservations:

Issues:

1) Since LzTextScrollEvent is so intimately tied with LzText, I would prefer it were _not_ in a separate file.  If we had inner classes, I would make it one.  We don't, but at least let's not put it in its own file because that gives the illusion it is standalone.

2) Rather that LzTextScrollEvent having to know all the the potential set of scroll event listeners, I suggest we just add a slot to LzText like `scrollEventListenerCount`.  Each time a LzTextScrollEvent gets a notify, it does

  scope.scrollEventListenerCount += (ready ? 1 : -1)
  var anyready =  scope.scrollEventListenerCount > 0;
  if (anyready != scope.scrollevents) {
    scope.$lzc$set_scrollevents(anyready);
  }

3) LzText#344 "NOTE: Using the onyscroll" instead of "using" say something like "a constraint on `yscroll`... or a handler for `onyscroll`... will automatically enable `scrollevents`.  Ditto for the other NOTEs in each of the attributes.  A constraint or handler will automatically enable updating of the attribute.  The only time you need to set scrollevents directly would be if you were, say, trying to poll the attribute without a constraint or handler (which would be very un-Laszlo-rific).

4) If you make LzTextDeclaredScrollEvent a static property of LzTextScrollEvent, then you should be able to declare the events in the class.  It really is a shame to re-initialize them in every instance.  If that doesn't work, just make it a global, like we do with LzDeclaredEvent.

5) Let's just delete `maxlines`.  It is an attractive nuisance.

Not approved yet.

On 2010-06-23, at 21:18, Max Carlson wrote:

> cChange 20100623-maxcarlson-H by maxcarlson at friendly on 2010-06-23 18:10:50 PDT
>    in /Users/maxcarlson/openlaszlo/trunk-clean
>    for http://svn.openlaszlo.org/openlaszlo/trunk
> 
> Summary: Automatically turn on scrollevents when dependent events are registered
> 
> Bugs Fixed: LPP-9146 - Use notifyingevents to automatically turn on scroll events on text
> 
> Technical Reviewer: ptw
> QA Reviewer: hminsky
> 
> Details: LzText - Create shared LzDeclaredEventClass instance for use with events that need scrollevents turned on.  Update documentation.  Set scroll events in construct() to keep DHTML happy.  Remove unused methods.
> 
> LzTextscrollEvent - Registering for any dependent event turns on scroll events, and unregistering checks for other dependent events with listeners before unregistering.
> 
> Library - Include LzTextscrollEvent
> 
> newcontent/scrollingtext - No need to explicitly turn on scrollevents.
> 
> Tests: Debugger works as before.
> 
> Files:
> M       WEB-INF/lps/lfc/views/LzText.lzs
> A       WEB-INF/lps/lfc/views/LzTextscrollEvent.lzs
> M       WEB-INF/lps/lfc/views/Library.lzs
> M       lps/components/debugger/newcontent.lzx
> M       lps/components/debugger/scrollingtext.lzx
> 
> Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20100623-maxcarlson-H.tar