Sequence ordering issues

[gadicc]

Ref: https://github.com/gadicc/meteor-famous-views/commit/4ae0a0262e8f52a15a6b150f2aea804c0145b9e5#commitcomment-8683624

https://github.com/PEM--/fview-flexgrid

[gadicc]

@PEM-- , does this definitely work in vanilla famous? not having any luck, even if I call sequenceFrom() after the children are in the array. can you give me an example of the ordering you use where it does work?

[PEM--]

It works in a Pen using only CS, jQuery and Famous: http://codepen.io/PEM--/pen/jEOJpg?editors=001

My first assumption was that the sequenceFrom should be called after the surface are added. I'm wrong. In vanilla famo.us, you can add an empty array of surfaces via the sequenceFrom, then add surfaces one by one.

I don't get what make a difference to what famousEach is doing and the vanilla famous.us approach.

Now, you understand why I've started investigating into the code :wink: Unfortunately, without success so far.

[gadicc]

Ok weird, not sure why I couldn't get it to work in vanilla famous, I guess with all the coffee script conversions something went wrong :> In code pen it's beautiful!

You're right. It's the famousEach. This works:

  +FlexGrid id='container' marginTop=50 marginSide=50 gutterCol=40 gutterRow=20 itemSize="[150,100]"
    +Surface class='red-bg'
      p name
    +Surface class='red-bg'
      p name
    +Surface class='red-bg'
      p name

(need the options for them to not all be on top of each other).

Super confused. The array is identical in both cases. And works with all the other layouts as you saw. Will keep looking into this.

[PEM--]

itemSize is the only option required. Once the Surface are in the sequenceFrom, the FlexGrid add a Modifier used for positioning and triggering animations if the parent's size is changed.

[PEM--]

For CS, it could be a problem. The Pen is in CS but as Meteor is sometimes messing with the this context for hooking reactivity, this could put CS code in a weird state. That was one of my many other assumptions. I've put console.log @ everywhere for checking changes in context without luck.

Debugging famo.us is very uneasy in the DevTools. There are many functions called at each frame. Well at least, it's feasible when you compare to bugs in CSS. An advantage but... very thin.

[gadicc]

Just about worked this out... but struggling to follow all the CS hehe. Few things:

1) Ok no the HEAD of the repo is definitely broken. I didn't realize your symlinking structure, I was using the codepen code when I made my earlier comment... the repo as it is doesn't work in vanilla famous either, unless i'm doing the wrong options. So yeah with the codepen code I need to specify all the margin* etc options because they are all defaulted to undefined in the constructor.

2) Ok, it doesn't work with famousEach because of the timing. Without it the surfaces are already there for the first pass. _calcPositions for example is only called once, and with famousEach, ypos will be undefined so @height is NaN. Not sure how famo.us handle things like this, maybe check if the array size has changed and then rerun the stuff?

[gadicc]

e.g. _calcPositions is rerun when the context size changes.... so if you resize the browser window you'll see all the surfaces pop up (from #famousEach) that were hidden before. I guess it's that whole block. You could probably just add a condition to in addition to caching and checking context size, you check array size.

[gadicc]

Yup, here we go :)

    commit: (context) ->
      width = context.size[0]
      specs = []
      unless @_cachedWidth is width and @_cachedLength is @_items.length
        spacing = @_calcSpacing width
        size = @options.itemSize
        if spacing.numCols < 2
          spacing.numCols = 1
          spacing.marginSide = 0
          size = [width, size[1]]
        positions = @_calcPositions spacing
        for i in [0...@_items.length]
          if @_modifiers[i] is undefined
            @_createModifier i, positions[i], size
          else
            @_animateModifier i, positions[i], size
        @_cachedWidth = width
        @_cachedLength = @_items.length
      for i in [0...this._modifiers.length]
        spec = @_modifiers[i].modify target: @_items[i].render()
        specs.push spec
      specs

that works with the code I copied over from the codepen.

[PEM--]

Wow, you mean I was digging in the very wrong direction.

I really should not go to bed that early. You are more awake than me with much less sleep.

[PEM--]

I see that you are getting your hand on CS :smile: Pretty clean isn't it?

[gadicc]

Haha. DO NOT COPY MY SLEEP PATTERNS. I have serious problems :> The more I work (or more excited I am about a project), the worse I sleep. I generally try stop working by 7 (don't always succeed), and then hope I sleep until around 8. Some of those "late nights" you saw me at were actually just super early mornings (like 1:30am today). It's not sustainable nor healthy :P

CS is very nice and clean but I am not getting the hang of it at all. I can just look at JS and see exactly what's going on. With CS it takes effort and I still use js2coffee.org often. But I'll get there. Jade I'm enjoying a lot... especially when I learn new things. Some of the very cool stuff is identical in CS, I admit. One day :)

And don't stress, I often dig in the wrong direction :>

[gadicc]

This is going to be a winning plugin! It's super cool :)

[PEM--]

Check this one from Hein: https://github.com/IjzerenHein/famous-flex

I has simply tied all layout into one. You can change the layout dynamically. Jaw dropping.

[gadicc]

Ooh. That looks awesome! Yeah we must wrap that for sure. All those slider controls look like good candidates for reactivity. And actually, if I'm not mistaken, by default we get reactivity on all attributes through the setOptions method without doing anything - unless we override with a custom attrUpdate method when we wrap the view.

Although Famo.us are also working on all this stuff... I saw in the videos... but never see any code :)

[PEM--]

For CS, it takes some time to get used to. Some says around a week.

Once done, it divides by 2 the amount of code to write and you don't loose time on counting parenthesis anymore.

But I must admit that there is not a single day where I go back to JS 5 to 10 minutes to figure how I should write it in CS. In the end, CS buy me some time but it's uneasy to replace 15 years of JS. I guess that I'm still thinking in JS.

[PEM--]

The DEFAULT_OPTIONS is usable. I was thinking of doing something with it. Just like the Lightbox demo that famo.us puts on CodePen. Making it reactive for easier tweaking on screen. That could be done by finishing the ShowTree method that you've started. It would be easier to set values like damping and so on.

[PEM--]

I still don't get why they are hiding their code... When they will release it, inspect element and it's done.

Except if they go the way of code obfuscation... I've done it in the past. Customer's requirements.

[gadicc]

I actually almost did something with the lightbox until I realized it was based on a much earlier version of famo.us. and i guess their new one is based on mixed mode i think.

Heh I didn't get very far with showTree lol. Once I got the basic famous-views working properly, I didn't really have the need for it anymore, because the template structure makes it super clear what's going on, especially with jade!

[PEM--]

Yep with Jade, it's almost a doublon. But I think there is something to do with it. You could get all the options of the displayed components and put some kind of meta adjustment widget for tweaking every animations. Just like Famo.us is doing it for tweaking their demo on their widget but this time at the scale of the complete app. When synchronizing animations just like in the Timbre menu, it could be very useful.

[PEM--]

And hop, now we got a plugin for a reactive favico: https://atmospherejs.com/pierreeric/rxfavico.

That will help checking the test results.

[PEM--]

You put me on the right track. I completely forgot to add a Scrollview for embedding the FlexGrid.

Still, it doesn't work. I've put console.log on the Pen to check if getSize, render and commit functions were called properly at each frame by the Engine. They are.

Now, in the plugin, I've put the same console.log. Only the render and the commit are called at each frame by the Engine.

That is odd. I've updated the repo in case you would have another of your nice hints.

[gadicc]

Awesome work with the reactive favico! That's really cool!

Mmm, yeah... we could expose a way to tweak every attribute... it's just hard to guess the scale / step count. We'd have to look at the name as well and hard code each one in.

Wait so you're still not working? As I said, I copied the codepen code into the plugin. I'm not sure what was different, but the original plugin code never worked for me. If you like I can diff what I had in the end to I guess the updated repo.

[PEM--]

Still not working. But don't worry, I will sort it out. I've messed up with something and, suprisingly, it's teaching me a lot as I need to dig deeper in Meteor and Famo.us.

[PEM--]

Nailed some bits of it. Unfortunately, I still need an initial resize event. I keep digging.

[PEM--]

On the Pen, I've set up a console.log at time intervals. Here is what it shows:

scrollview [undefined, 0]
flexGrid [586, 6080]

On the example, here is what the same code shows:

fscrollview [undefined, 0]
fflexgrid undefined

The size within the grid remains undefined. If I resize the browser window, the size ends up calculated and the flexgrid appears. I could fake a resize event but I prefer carry on digging.

[PEM--]

Well, the only difference I can see is that the FlexGrid is getting an Array of MeteorFamousView in my example whereas the Pen receives an Array of Surface.

[PEM--]

In the example, the Surfaces in the MeteorFamousView wrapper are set with a _sizeDirty=true therefore they have a _size set to null.

In the Pen, the Surfaces are set with a _sizeDirty=false and have a proper _size set to the expected recalculated size ([486, 150]).

Sounds like the sequencer isn't transmitting the right object.

[PEM--]

I got it working, but with an hideous hack... Still, it's far from being perfect and reveals some issue with size calculations on the parent Scrollview :expressionless:

This is exactly the case on our main site. When you choose a Template which contains a Scrollview and when you choose the language Jade, the size of the snippet is reduced. But the Scrollview does not take it into account leading to blank surface or scroll completely lost.

I think we have a nice issue on the _sizeDirty (and others dirtyfication done in famo.us). ElementOutput are not taken into account when set to dirty thus leading to size or transform recalculation issue.

[PEM--]

The hideous hack:

  FView.registerView 'FlexGrid', FlexGrid,
    famousCreatedPost: ->
      node = @parent
      node = node.parent until node.id? and node.id is 'mainCtx'
      [width, height] = node.context._size
      famous.utilities.Timer.setTimeout ->
        node.context.setSize [width+1, height+1]
      , 500

[gadicc]

Wait, but I'm not sure why that's necessary at all? After putting in the code from codepen, I think I only changed like 3 lines of code to get everything working.

You can clone https://github.com/gadicc/fview-flexgrid.git to see, I also diffed it against your latest commit here, but unfortunately a lot of changes has happened since then, so it's not as clear a diff as I would have liked.

I left in all the debug stuff I put it so it might be clearer what's going on. Also unfortunately since it's based on the earlier code, I still have all the defaults on the helper call since in the original file it was all undefined.

Hope this helps! Let me know what the final issue was :>

Re height on the site, that's unfortunate. We used to use the autoHeight() stuff and it worked excellently. I switched it back to using true height with the new famo.us, I guess it never checks the size again (which is smart), but we need to retrigger the size check (shouldn't be hard). Tracking here: https://github.com/gadicc/meteor-famous-views/issues/163.

[PEM--]

OK, roger that. I will check your fork project. As always, I'm very thankful for your help and always amazed by your cleverness.

What I'm not getting is that getSize, getOpacity, getTransrom, renderand commit should do their role when the scene graph is traversed. My trick to retrigger it makes me think that there is 2 mechanism of traversal:

• one by the Entity (the id) when the context has changed. And we are compatible with this one.
• one at the instanciation where all descendent children are checked for establishing the appropriate size of the parent (the size = true for the parent).

My two cents is that we are disturbing it the second one by introducing a MeteorFamousView in the middle. Mapping MeteorFamousView's member function to its inner content may be the missing piece.

[gadicc]

Just looked at the diff. Not 3 lines, 2 lines :) Particularly the change from https://github.com/gadicc/fview-flexgrid/commit/3c5312ac1d447a815fa509d241d17fe854612541#diff-4054845b51edbd3292f1fb25992c8bb4L98 to the next line, and the addition of https://github.com/gadicc/fview-flexgrid/commit/3c5312ac1d447a815fa509d241d17fe854612541#diff-4054845b51edbd3292f1fb25992c8bb4R105.

Not everything gets re-run on every tick... it would be too slowly. Generally famous caches things and marks as dirty things that need to be recalculated, so it only does these calculations when required, not 60 times a second :>. The flexgrid is the perfect example, and we just said, don't recalculate just when the containing context side is changed, also recalculate when the number of items changes.

MeteorFamousView is basically a RenderNode with some extra properties. It maps getSize() and render(). I see now RenderNode has commit() instead of render(), maybe it always did, but we don't really need all that stuff. Anyways, our benchmark should be that if something works in vanilla famo.us, it should work in in famous-views. And if that's not the case, either we have something that's broken, or we need to prove that something we're doing is triggering a less-obvious bug in famo.us.

[PEM--]

Yep, 2 lines circumvented it: render is modified so that it takes it checks the items.lengthand uses it to force a rendering. Thanks a lot for your insights.

I have carefully re-read my Pen. The Pen is a vanilla famo.us. It doesn't need this forced recalculation. I'm sure that there is something wrong in our current implementation. Unfortunately, I'me just wild guessing on what makes the first render not taking the height into account. What I've found (the hideous hack), it that when you modify the size of the parent context, everything goes back to normal. This size modification has to be done with a setTimeout for being taken into account.

Well pretty weird my friend. I think we will have to set additional tests to pinpoint what we are not doing and is requested by famo.us.

[gadicc]

when you modify the size of the parent, it causes positions to be recalculated. you also want that to happen if the number of items change. there's no other way around it :> in vanilla famous adding new items at a later time won't work either, try it.

[PEM--]

Dawn! It's me again. It all boils down owing to the way Surfaces are added. In the vanilla famo.us, the for loop is interpreted in a single Engine tick. That's why. It's the main difference with the parsing induced afterward in the famousEach.

Wahou, so simple. I should have get that :disappointed:

[gadicc]

Which for loop? It's true, when adding stuff via a templating approach, we're more likely to encounter problems than vanilla famo.us users. but that's not because our code is buggy, we're just more likely to run into issues that would affect famo.us anyway. in this flexgrid example, in vanilla famo.us, maybe no one would notice a problem since they add all the elements before the loop is run. but that doesn't mean the flexgrid shouldn't allow elements to be added/removed at a later time.

it also varies by view. a sequential layout for example does actually recalculate everything on each commit, because it's possible the size of the surfaces inside of it have changed. in this case, it relies on each surface to cache it's own size, and report back the cached value and only recalculate when necessary. in flexgrid, since the sizes are fixed, these calculations happen in the view itself, but are still cached to only be rerun if necessary.

tale a look at e.g. the commit of a surface, you'll see that it only recalculates things as necessary... when 1) the factors needed to calculate the value have chagned and 2) the value is needed. otherwise, no point in doing the calculations and wasting CPU cycles.

[PEM--]

Indeed the code is now more robust and not restrained to a fixed number of items at instantiation.

As always, you've made me learn a ton. I'm just feeling bad to have waste your time on stuff that I should have understand. I hope that I will be able to pay you back on this.

[gadicc]

Don't stress, you are helping loads with other things :) We all have to learn at some point. You should have seen some of my early packages for Meteor haha. The road to becoming a pro is not an easy one, but you're on it now :)