Thank you for the review, Piotr. 1) Done, thanks for noticing. 2) scrollTimerHandler() only nulls out the _scrollTimer temporarily, and then puts it back in the finally clause. It looks like even if there's an error thrown in interactionManager.mouseMoveHandler, the finally clause will still run and re-assign the _scrollTimer: "_scrollTimer = stashedScrollTimer;" Given all this, I don't think it's a good idea to create a separate function just for nulling the _scrollTimer. Do you? 3) also made some small improvements (like optimising imports) in StandardFlowComposer and ContainerController.
Finally, any thoughts about the unit/mustella test we could build around this? Thanks, Mihai On 23 October 2014 06:31, piotrz <[email protected]> wrote: > Hi Mihai, > > It looks really good for me! I am also not an expert in TLF, but what you > did looks reasonable. > I have two comments: > > 1) In your function stopMouseSelectionScrolling I would use for condition > curly brackets. > Why? I think it's more readable, and such place is become less I would say > expose on error. > I saw so many times in my inspected code that devs did mistakes with > condition without curly brackets. > Ex: > > If (something) > oldcode; > my new code should be inside condition but I forgot to add curly brackets; > > I hope you understand what I mean. > > 2) In your function you are removing _scrollTimer and it's good, but we have > such removes also in method scrollTimerHandler, but without removing > registered event. > > Let's create a function for removing this scroll timer and use this function > inside stopMouseSelectionScrolling and scrollTimerHandler. > > This function should remove registered event listener and nulling our timer. > > Thanks, > Piotr > > > > > > ----- > Apache Flex PMC > [email protected] > -- > View this message in context: > http://apache-flex-development.2333347.n4.nabble.com/Re-git-commit-flex-tlf-refs-heads-develop-FLEX-26478-CAUSE-When-replacing-ContainerControllers-trigge-tp41683p41696.html > Sent from the Apache Flex Development mailing list archive at Nabble.com.
