I started on this. I was doing two things: 1. I added a visible check to only trigger layout if visible is true. There are very few places this is needed. It’s not a lot of extra code. 2. I started adding “show” event listeners. During the process I noticed there is already a “LayoutOnShow” bead which injects a “show” event listener which in turn dispatches a layoutNeeded event.
The bead is probably more PAYG, but it’s also more “pay” when you do go. Changing bead notifications would probably make it cheaper though. So, I’m going to keep #1 (because I don’t see any other straight-forward way of avoiding the unnecessary layouts), but not do #2. The question I have is whether to add the LayoutOnShow bead to the defaults for Basic, or should that only go into Express? Harbs > On Aug 3, 2017, at 11:57 PM, Harbs <harbs.li...@gmail.com> wrote: > > My ideas on bead lifecycles might help for this. Not sure. > > I’m not sure there’s a perfect solution to this problem. > > If I have to weigh a single check for visible vs an entire layout, I’d go for > the former. > > I seem to recall that we did something to prevent non-visible components for > going throw layout, but I don’t remember any details. Maybe Yishay knows what > I’m talking about. > >> On Aug 3, 2017, at 7:18 PM, Alex Harui <aha...@adobe.com.INVALID> wrote: >> >> Yeah, but I also remembered on other thing. The vast majority of >> components are never made invisible, so adding a check in each layout bead >> just-in-case they are made invisible isn't very PAYG. >> >> So maybe there is some other way that setting visible=false can inject >> code that handles sharing the CSS display property with the layouts. Or >> maybe it isn't worth worrying about right now. >> >> My 2 cents, >> -Alex >> >> On 8/3/17, 8:50 AM, "Harbs" <harbs.li...@gmail.com> wrote: >> >>> But it it doesn’t set display, we’re going to have to run layout every >>> time the visibility changes. >>> >>> I’ve already made my changes. I’ll commit soon. >>> >>> Ah. I see what you mean. By doing it your way, there’s no reason to >>> actually run layout until (or if) the visibility is set to true. >>> >>> That probably *is* better. I’ll commit what I did for now, because it at >>> least fixes the bug, and when I have time, I’ll look into improving the >>> layouts. >>> >>> Another thought: >>> >>> If visibility is turned on and off multiple times, this might be less >>> efficient, but the layout might be able to store a flag to avoid running >>> the layout if not needed… >>> >>> Harbs >>> >>>> On Aug 3, 2017, at 6:24 PM, Alex Harui <aha...@adobe.com.INVALID> wrote: >>>> >>>> Right, so layout code would have to check for display=="none" and not >>>> set >>>> display and listen for the show event. >>>> >>>> Maybe as you clean up the setting of display multiple times it will be >>>> come clear as to whether listening for "show" is cheaper/cleaner than >>>> displayStyleForLayout. >>>> >>>> -Alex >>>> >>>> On 8/3/17, 8:18 AM, "Harbs" <harbs.li...@gmail.com> wrote: >>>> >>>>> The problem is that visible is set before the bead exists. >>>>> >>>>> BTW, Some of the layout seem to be reading and setting display multiple >>>>> times. That can cause layout thrashing. That should probably be >>>>> resolved. >>>>> >>>>>> On Aug 3, 2017, at 6:05 PM, Alex Harui <aha...@adobe.com.INVALID> >>>>>> wrote: >>>>>> >>>>>> FWIW, I'm not sure this is the best pattern. It was what we did to >>>>>> get >>>>>> the examples to run. >>>>>> >>>>>> Another option is that layout beads listen for changes to visible and >>>>>> reset the CSS display style when visible changes. >>>>>> >>>>>> Food for thought, >>>>>> -Alex >>>>>> >>>>>> On 8/3/17, 8:00 AM, "Harbs" <harbs.li...@gmail.com> wrote: >>>>>> >>>>>>> I’m using a VerticalFlexLayout in a component. Under certain >>>>>>> circumstances, I need to set the visibility of the component to >>>>>>> false. >>>>>>> >>>>>>> These two settings are contradictory in JS. >>>>>>> >>>>>>> visible=false sets display to none >>>>>>> VerticalFlexLayout sets the display to flex >>>>>>> >>>>>>> When setting visible to false, it uses a property >>>>>>> “displayStyleForLayout” >>>>>>> to store the value set by the layout. The problem is that the layout >>>>>>> might be applied AFTER visible is already set to false (which is the >>>>>>> case >>>>>>> in my situation). >>>>>>> >>>>>>> I’m thinking of solving this by exposing displayStyleForLayout so it >>>>>>> can >>>>>>> be set by beads if necessary. >>>>>>> >>>>>>> Scratch that. I see that already exists. Problem solved… I’ll fix the >>>>>>> layouts. >>>>>>> >>>>>>> Harbs >>>>>> >>>>> >>>> >>> >> >
