Skip to content

Commit

Permalink
more robust layer transitions
Browse files Browse the repository at this point in the history
  • Loading branch information
@lexoyo
lexoyo committed Oct 12, 2012
1 parent e54ac88 commit a067c5e
Showing 1 changed file with 9 additions and 9 deletions.
18 changes: 9 additions & 9 deletions src/brix/component/navigation/Layer.hx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,6 @@ with sum of the css classes
*/
public function show(transitionData:Null<TransitionData> = null, preventTransitions:Bool = false) : Void
{
if (status != hidden && status != notInit)
{
trace("Warning: can not show the layer, since it has the status '"+status+"'");
return;
}
// reset transition if it is pending
if (status == hideTransition)
{
Expand All @@ -255,6 +250,11 @@ with sum of the css classes
doShowCallback(null);
removeTransitionEvent(doShowCallback);
}
if (status != hidden && status != notInit)
{
trace("Warning: can not show the layer, since it has the status '"+status+"'");
return;
}
// update status
status = showTransition;

Expand Down Expand Up @@ -331,10 +331,6 @@ with sum of the css classes
*/
public function hide(transitionData:Null<TransitionData> = null, preventTransitions:Bool) : Void
{// trace("hide "+preventTransitions);
if (status != visible && status != notInit){
//trace("Warning, can not hide the layer, since it has the status '"+status+"'");
return;
}
// reset transition if it is pending
if (status == hideTransition){
trace("Warning: hide break previous transition hide");
Expand All @@ -347,6 +343,10 @@ with sum of the css classes
doShowCallback(null);
removeTransitionEvent(doShowCallback);
}
if (status != visible && status != notInit){
trace("Warning, can not hide the layer, since it has the status '"+status+"'");
return;
}
// update status
status = hideTransition;

Expand Down

5 comments on commit a067c5e

@zabojad
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alex,

The styleAttrDisplay you've added breaks the possibility to run the Layer component at macro or PHP time. I'd remove it or find another way to do what you wanted to do...

Generally, we should try to make our components server and macro time compatible (so think what would be its behavior and what would look like the DOM after a pre-execution of the Brix application).

Can you change this styleAttrDisplay mecanism in that way ?

Thomas

@lexoyo
Copy link
Member Author

@lexoyo lexoyo commented on a067c5e Oct 22, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm, what do you mean, i don't see any styleAttrDisplay in the code

@zabojad
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uuups, indeed, it's something that seems to be there since the beginning...

Just look at the code of Layer: https://github.com/silexlabs/Brix/blob/develop/src/brix/component/navigation/Layer.hx

We have a styleAttrDisplay variable that keeps the value of the "original" style="display:..." value. But it doesn't actualy keeps the true value of the display style attribute, it keeps instead the one set in the HTML of the document (if any). This is a problem since if the application is executed at PHP time or macro time as we plan to do, styleAttrDisplay will be always "none" for other pages than the initial page.

I think we shouldn't have this styleAttrDisplay variable but instead, we should do rootElement.style.display=null; when showing the Layer. Why ? because this wouldn't override the value of the display attribute that would be in external css file and would just delete the style="display:..." that is in the DOM tree.

Do you see what I mean ?

@lexoyo
Copy link
Member Author

@lexoyo lexoyo commented on a067c5e Oct 23, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is important to keep the display value
Why is it a problem in php/macro?

@zabojad
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, in HTML/js (If the behavior is different with cocktail, we should perhaps fix it in cocktail), when setting element.style.display ="something", it sets the display style attribute right into the DOM tree (in the runtime HTML when you analyze with firebug). It takes precedence over any value that could have been set in linked css files and override any value that could have been set directly in the HTML document (with style="...").

But when doing element.style.display = null, it removes any display style value set in the runtime HTML, but keeps and eventualy restore any potential display value set in linked css files (if no display value is set for the element in linked css files, it will restore its default display value).

You see the difference? So I think we should not try to keep the display value in a variable (as anyway, we would keep only the one that could have been set directly in the HTML file), but instead set it to none when hiding the layer and set it to null when showing it (as this will just restore the display value that is potentially set in a linked css file and if not, restore the default display value for the element).

Please sign in to comment.