flex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Harui <aha...@adobe.com.INVALID>
Subject Re: [2/4] git commit: [flex-asjs] [refs/heads/develop] - LayoutBase Fix for strand set to null in remove bead, plus performance improvements in js
Date Sat, 10 Jun 2017 03:59:04 GMT
Yes, thanks for removing unnecessary coercions.

While debugging examples, I also noted the removal of Layout in Panel.
Makes me wonder if we our container architecture/lifecycle is designed
properly.  The base classes may need to have a pattern for "proxies" and
"wrappers", which may be the essence of what Panels and Scrolling
Containers do (along with exposing instead of hiding the
addElement/removeElement APIs).

Looking at the code for bead removal, it seems so heavy it makes you want
to avoid it all together.  For the password input bead, for example, we
know there can be a version of the bead with a flag to enable/disable the
password characters.  How heavy is that implementation vs the one that
requires removal?  For Panel, maybe the lifecycle should just not assume
that the Container has a layout bead so there isn't one to replace.

Food for thought,
-Alex

On 6/9/17, 4:00 PM, "Greg Dove" <greg.dove@gmail.com> wrote:

>I happened to be testing against FlexJSStore at the time I discovered the
>need for this, and there is some removing going on.
>
>I think I improved performance compared to how it was originally by
>removing coercion in js and using local references for some of the repeat
>listener references/avoiding repeat closure lookups on js, so it is
>probably 'less' PAYG than it was before, with more safety, but I get the
>point.
>
>I'm not sure about injection, but I will come back to this and add a
>removable layout subclass for the FlexJSStore demo (not sure if the
>PanelView or whatever seems to be doing this is also used in other
>examples, I have not checked).
>
>
>
>
>On Sat, Jun 10, 2017 at 10:45 AM, Alex Harui <aharui@adobe.com> wrote:
>
>> I guess I don't believe that removing beads is so common that every app
>> needs to carry this code around.  I wonder if there is a way to inject
>> removability as needed.
>>
>> Having a RemovableXXXLayout could override the strand setter and remove
>> listeners if the handlers are protected.
>>
>> Thoughts?
>> -Alex
>>
>> On 6/8/17, 3:22 PM, "gregdove@apache.org" <gregdove@apache.org> wrote:
>>
>> >LayoutBase Fix for strand set to null in remove bead, plus performance
>> >improvements in js
>> >
>> >
>> >Project:
>> >https://na01.safelinks.protection.outlook.com/?url=
>> http%3A%2F%2Fgit-wip-us
>> >.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Frepo&data=02%
>> 7C01%7C%7Cb863f5cff77
>> >e41e305b708d4aebcd62e%7Cfa7b1b5a7b34438794aed2c178de
>> cee1%7C0%7C0%7C6363255
>> >73442866659&sdata=NsSQs6LJBQaYIJ36tZ%2FgMmMyuffAlo7pAuwtopOok2g%3D&
>> reserve
>> >d=0
>> >Commit:
>> >https://na01.safelinks.protection.outlook.com/?url=
>> http%3A%2F%2Fgit-wip-us
>> >.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Fcommit%
>> 2F08af60c7&data=02%7C01%7C%
>> >7Cb863f5cff77e41e305b708d4aebcd62e%7Cfa7b1b5a7b34438794aed2c178de
>> cee1%7C0%
>> >7C0%7C636325573442866659&sdata=a8HJuqkEEii03BRyFH6lMcvkLFirlc
>> lo6HwG%2F6w4J
>> >kA%3D&reserved=0
>> >Tree:
>> >https://na01.safelinks.protection.outlook.com/?url=
>> http%3A%2F%2Fgit-wip-us
>> >.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Ftree%2F08af60c7&
>> data=02%7C01%7C%7C
>> >b863f5cff77e41e305b708d4aebcd62e%7Cfa7b1b5a7b34438794aed2c178de
>> cee1%7C0%7C
>> >0%7C636325573442866659&sdata=CcHeFSf6IMQ75kKXSE%
>> 2BWY23J7VrbJauoxO1TvG%2BHS
>> >Yk%3D&reserved=0
>> >Diff:
>> >https://na01.safelinks.protection.outlook.com/?url=
>> http%3A%2F%2Fgit-wip-us
>> >.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Fdiff%2F08af60c7&
>> data=02%7C01%7C%7C
>> >b863f5cff77e41e305b708d4aebcd62e%7Cfa7b1b5a7b34438794aed2c178de
>> cee1%7C0%7C
>> >0%7C636325573442866659&sdata=E%2B6iEUFqZB3%2BO15%
>> 2BK3sHlnaiuIkWgm8DvWVcmUm
>> >nnIk%3D&reserved=0
>> >
>> >Branch: refs/heads/develop
>> >Commit: 08af60c7755a9c7dd64ab62cbfff97443841bda7
>> >Parents: b0f7013
>> >Author: greg-dove <greg.dove@gmail.com>
>> >Authored: Fri Jun 9 10:07:20 2017 +1200
>> >Committer: greg-dove <greg.dove@gmail.com>
>> >Committed: Fri Jun 9 10:17:54 2017 +1200
>> >
>> >----------------------------------------------------------------------
>> > .../flex/org/apache/flex/core/LayoutBase.as     | 40
>> +++++++++++++++-----
>> > 1 file changed, 30 insertions(+), 10 deletions(-)
>> >----------------------------------------------------------------------
>> >
>> >
>> >https://na01.safelinks.protection.outlook.com/?url=
>> http%3A%2F%2Fgit-wip-us
>> >.apache.org%2Frepos%2Fasf%2Fflex-asjs%2Fblob%2F08af60c7%
>> 2Fframeworks%2Fpro
>> >jects%2FBasic%2Fsrc%2Fmain%2Fflex%2Forg%2Fapache%2Fflex%
>> 2Fcore%2FLayoutBas
>> >e.as&data=02%7C01%7C%7Cb863f5cff77e41e305b708d4aebc
>> d62e%7Cfa7b1b5a7b344387
>> >94aed2c178decee1%7C0%7C0%7C636325573442866659&sdata=
>> fnKUKGhxZfwPRFkiMwfiri
>> >aNYV21HE8X7463sLtyYOo%3D&reserved=0
>> >----------------------------------------------------------------------
>> >diff --git
>> >a/frameworks/projects/Basic/src/main/flex/org/apache/flex/
>> core/LayoutBase.
>> >as
>> >b/frameworks/projects/Basic/src/main/flex/org/apache/flex/
>> core/LayoutBase.
>> >as
>> >index adadc75..be7d642 100644
>> >---
>> >a/frameworks/projects/Basic/src/main/flex/org/apache/flex/
>> core/LayoutBase.
>> >as
>> >+++
>> >b/frameworks/projects/Basic/src/main/flex/org/apache/flex/
>> core/LayoutBase.
>> >as
>> >@@ -76,19 +76,39 @@ package org.apache.flex.core
>> >          *  @productversion FlexJS 0.8
>> >                *
>> >                * @flexjsignorecoercion
>>org.apache.flex.core.ILayoutChild
>> >+               * @flexjsignorecoercion org.apache.flex.events.
>> IEventDispatcher
>> >          */
>> >               public function set strand(value:IStrand):void
>> >               {
>> >-            host = value as ILayoutChild;
>> >-
>> >-                      IEventDispatcher(host).addEventListener("
>> widthChanged",
>> >handleSizeChange);
>> >-                      IEventDispatcher(host).addEventListener("
>> heightChanged",
>> >handleSizeChange);
>> >-                      IEventDispatcher(host).
>> addEventListener("sizeChanged",
>> >handleSizeChange);
>> >-
>> >-                      IEventDispatcher(host).addEventListener("
>> childrenAdded",
>> >handleChildrenAdded);
>> >-                      IEventDispatcher(host).addEventListener("
>> initComplete",
>> >handleInitComplete);
>> >-
>> >-                      IEventDispatcher(host).addEventListener("
>> layoutNeeded",
>> >handleLayoutNeeded);
>> >+                      var newHost:ILayoutChild = value as
>>ILayoutChild;
>> >+                      var oldHost:ILayoutChild = host;
>> >+                      if (newHost != oldHost) {
>> >+                              var sizeChange:Function =
>>handleSizeChange;
>> >+                var childrenAdded:Function =handleChildrenAdded;
>> >+                              var initComplete:Function =
>> handleInitComplete;
>> >+                              var layoutNeeded:Function =
>> handleLayoutNeeded;
>> >+                if (oldHost) {
>> >+
>> >IEventDispatcher(oldHost).removeEventListener("widthChanged",
>> sizeChange);
>> >+
>> >IEventDispatcher(oldHost).removeEventListener("heightChanged",
>> >sizeChange);
>> >+
>> >IEventDispatcher(oldHost).removeEventListener("sizeChanged",
>>sizeChange);
>> >+
>> >+
>> >IEventDispatcher(oldHost).removeEventListener("childrenAdded",
>> >childrenAdded);
>> >+
>> >IEventDispatcher(oldHost).removeEventListener("initComplete",
>> >initComplete);
>> >+
>> >+
>> >IEventDispatcher(oldHost).removeEventListener("layoutNeeded",
>> >layoutNeeded);
>> >+                }
>> >+                              host = newHost;
>> >+                              if (newHost) {
>> >+
>> >IEventDispatcher(newHost).addEventListener("widthChanged", sizeChange);
>> >+
>> >IEventDispatcher(newHost).addEventListener("heightChanged",
>>sizeChange);
>> >+
>> >IEventDispatcher(newHost).addEventListener("sizeChanged", sizeChange);
>> >+
>> >+
>> >IEventDispatcher(newHost).addEventListener("childrenAdded",
>> >childrenAdded);
>> >+
>> >IEventDispatcher(newHost).addEventListener("initComplete",
>>initComplete);
>> >+
>> >+
>> >IEventDispatcher(newHost).addEventListener("layoutNeeded",
>>layoutNeeded);
>> >+                              }
>> >+                      }
>> >               }
>> >
>> >               /**
>> >
>>
>>

Mime
View raw message