Skip to content
This repository has been archived by the owner on Apr 20, 2023. It is now read-only.

Commit

Permalink
Combine the root renderer's position setting with its normal updating…
Browse files Browse the repository at this point in the history 
… process. Position/z-index/visibility properties are now built into a single CSS string and applied directly in the initial markup, or all at once as cssText in subsequent updates, which should make both faster. Also fixes issues with visibility not getting updated at proper times. Respond to style.* changes on watched ancestors.
  • Loading branch information
@lojjic
lojjic committed May 28, 2012
1 parent 3391158 commit 220dc17
Show file tree
Hide file tree
Showing 7 changed files with 130 additions and 147 deletions.
11 changes: 11 additions & 0 deletions sources/BoundsInfo.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,17 @@ PIE.BoundsInfo.prototype = {

_locked: 0,

/**
* Determines if the element's position has changed since the last update.
* TODO this does a simple getBoundingClientRect comparison for detecting the
* changes in position, which may not always be accurate; it's possible that
* an element will actually move relative to its positioning parent, but its position
* relative to the viewport will stay the same. Need to come up with a better way to
* track movement. The most accurate would be the same logic used in RootRenderer.updatePos()
* but that is a more expensive operation since it performs DOM walking, and we want this
* check to be as fast as possible. Perhaps introduce a -pie-* flag to trigger the slower
* but more accurate method?
*/
positionChanged: function() {
var last = this._lastBounds,
bounds;
Expand Down
47 changes: 17 additions & 30 deletions sources/Element.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ PIE.Element = (function() {

function Element( el ) {
var me = this,
renderers,
childRenderers,
rootRenderer,
boundsInfo = new PIE.BoundsInfo( el ),
styleInfos,
Expand All @@ -79,8 +79,7 @@ PIE.Element = (function() {
cs = el.currentStyle,
lazy = cs.getAttribute( lazyInitCssProp ) === 'true',
trackActive = cs.getAttribute( trackActiveCssProp ) !== 'false',
trackHover = cs.getAttribute( trackHoverCssProp ) !== 'false',
childRenderers;
trackHover = cs.getAttribute( trackHoverCssProp ) !== 'false';

// Polling for size/position changes: default to on in IE8, off otherwise, overridable by -pie-poll
poll = cs.getAttribute( pollCssProp );
Expand Down Expand Up @@ -160,7 +159,6 @@ PIE.Element = (function() {
}
rootRenderer.childRenderers = childRenderers; // circular reference, can't pass in constructor; TODO is there a cleaner way?
}
renderers = [ rootRenderer ].concat( childRenderers );

// Add property change listeners to ancestors if requested
initAncestorEventListeners();
Expand Down Expand Up @@ -229,28 +227,18 @@ PIE.Element = (function() {
if( initialized ) {
lockAll();

var i, len = renderers.length,
var i = 0, len = childRenderers.length,
sizeChanged = boundsInfo.sizeChanged();

for( i = 0; i < len; i++ ) {
renderers[i].prepareUpdate();
for( ; i < len; i++ ) {
childRenderers[i].prepareUpdate();
}
for( i = 0; i < len; i++ ) {
if( force || sizeChanged || ( isPropChange && renderers[i].needsUpdate() ) ) {
renderers[i].updateRendering();
if( force || sizeChanged || ( isPropChange && childRenderers[i].needsUpdate() ) ) {
childRenderers[i].updateRendering();
}
}
rootRenderer.finishUpdate();

if( force || ( ( !isPropChange || rootRenderer.isPositioned ) && boundsInfo.positionChanged() ) ) {
/* TODO just using getBoundingClientRect (used internally by BoundsInfo) for detecting
position changes may not always be accurate; it's possible that
an element will actually move relative to its positioning parent, but its position
relative to the viewport will stay the same. Need to come up with a better way to
track movement. The most accurate would be the same logic used in RootRenderer.updatePos()
but that is a more expensive operation since it does some DOM walking, and we want this
check to be as fast as possible. */
rootRenderer.updatePos();
if( force || sizeChanged || isPropChange || boundsInfo.positionChanged() ) {
rootRenderer.updateRendering();
}

unlockAll();
Expand All @@ -265,14 +253,12 @@ PIE.Element = (function() {
* Handle property changes to trigger update when appropriate.
*/
function propChanged() {
var e = event;

// Some elements like <table> fire onpropertychange events for old-school background properties
// ('background', 'bgColor') when runtimeStyle background properties are changed, which
// results in an infinite loop; therefore we filter out those property names. Also, 'display'
// is ignored because size calculations don't work correctly immediately when its onpropertychange
// event fires, and because it will trigger an onresize event anyway.
if( !( e && e.propertyName in ignorePropertyNames ) ) {
if( initialized && !( event && event.propertyName in ignorePropertyNames ) ) {
update( 1 );
}
}
Expand Down Expand Up @@ -341,7 +327,7 @@ PIE.Element = (function() {
*/
function ancestorPropChanged() {
var name = event.propertyName;
if( name === 'className' || name === 'id' ) {
if( name === 'className' || name === 'id' || name.indexOf( 'style.' ) === 0 ) {
propChanged();
}
}
Expand Down Expand Up @@ -399,12 +385,13 @@ PIE.Element = (function() {
destroyed = 1;

// destroy any active renderers
if( renderers ) {
for( i = 0, len = renderers.length; i < len; i++ ) {
renderers[i].finalized = 1;
renderers[i].destroy();
if( childRenderers ) {
for( i = 0, len = childRenderers.length; i < len; i++ ) {
childRenderers[i].finalized = 1;
childRenderers[i].destroy();
}
}
rootRenderer.destroy();

// Remove from list of polled elements in IE8
if( poll ) {
Expand All @@ -414,7 +401,7 @@ PIE.Element = (function() {
PIE.OnResize.unobserve( update );

// Kill references
renderers = boundsInfo = styleInfos = styleInfosArr = el = null;
childRenderers = rootRenderer = boundsInfo = styleInfos = styleInfosArr = el = null;
me.el = me = 0;
}
}
Expand Down
2 changes: 1 addition & 1 deletion sources/IE9BorderImageRenderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ PIE.IE9BorderImageRenderer = PIE.RendererBase.newRenderer( {
// will have been created asynchronously after the main element's update has finished; we'll
// therefore need to force the root renderer to sync to the final background once finished.
if( isAsync ) {
me.parent.finishUpdate();
me.parent.updateRendering();
}
}, me );

Expand Down
6 changes: 1 addition & 5 deletions sources/IE9RootRenderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@
*/
PIE.IE9RootRenderer = PIE.RendererBase.newRenderer( {

updatePos: PIE.emptyFn,
updateRendering: PIE.emptyFn,
updateVisibility: PIE.emptyFn,

outerCommasRE: /^,+|,+$/g,
innerCommasRE: /,+/g,

Expand All @@ -19,7 +15,7 @@ PIE.IE9RootRenderer = PIE.RendererBase.newRenderer( {
bgLayers[zIndex] = bg || undef;
},

finishUpdate: function() {
updateRendering: function() {
var me = this,
bgLayers = me._bgLayers,
bg;
Expand Down
187 changes: 89 additions & 98 deletions sources/RootRenderer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,6 @@
*/
PIE.RootRenderer = PIE.RendererBase.newRenderer( {

/**
* Flag indicating the element has already been positioned at least once.
* @type {boolean}
*/
isPositioned: false,

isActive: function() {
var children = this.childRenderers;
for( var i in children ) {
Expand All @@ -22,72 +16,58 @@ PIE.RootRenderer = PIE.RendererBase.newRenderer( {
return false;
},

needsUpdate: function() {
return this.styleInfos.visibilityInfo.changed();
},

/**
* Tell the renderer to update based on modified element position
*/
updatePos: function() {
if( this.isActive() && this.getBoxEl() ) {
var el = this.getPositioningElement(),
par = el,
docEl,
parRect,
tgtCS = el.currentStyle,
tgtPos = tgtCS.position,
boxPos,
s = this.getBoxEl().style, cs,
x = 0, y = 0,
elBounds = this.boundsInfo.getBounds(),
logicalZoomRatio = elBounds.logicalZoomRatio;

if( tgtPos === 'fixed' && PIE.ieVersion > 6 ) {
x = elBounds.x * logicalZoomRatio;
y = elBounds.y * logicalZoomRatio;
boxPos = tgtPos;
getBoxCssText: function() {
var el = this.getPositioningElement(),
par = el,
docEl,
parRect,
tgtCS = el.currentStyle,
tgtPos = tgtCS.position,
boxPos,
cs,
x = 0, y = 0,
elBounds = this.boundsInfo.getBounds(),
vis = this.styleInfos.visibilityInfo.getProps(),
logicalZoomRatio = elBounds.logicalZoomRatio;

if( tgtPos === 'fixed' && PIE.ieVersion > 6 ) {
x = elBounds.x * logicalZoomRatio;
y = elBounds.y * logicalZoomRatio;
boxPos = tgtPos;
} else {
// Get the element's offsets from its nearest positioned ancestor. Uses
// getBoundingClientRect for accuracy and speed.
do {
par = par.offsetParent;
} while( par && ( par.currentStyle.position === 'static' ) );
if( par ) {
parRect = par.getBoundingClientRect();
cs = par.currentStyle;
x = ( elBounds.x - parRect.left ) * logicalZoomRatio - ( parseFloat(cs.borderLeftWidth) || 0 );
y = ( elBounds.y - parRect.top ) * logicalZoomRatio - ( parseFloat(cs.borderTopWidth) || 0 );
} else {
// Get the element's offsets from its nearest positioned ancestor. Uses
// getBoundingClientRect for accuracy and speed.
do {
par = par.offsetParent;
} while( par && ( par.currentStyle.position === 'static' ) );
if( par ) {
parRect = par.getBoundingClientRect();
cs = par.currentStyle;
x = ( elBounds.x - parRect.left ) * logicalZoomRatio - ( parseFloat(cs.borderLeftWidth) || 0 );
y = ( elBounds.y - parRect.top ) * logicalZoomRatio - ( parseFloat(cs.borderTopWidth) || 0 );
} else {
docEl = doc.documentElement;
x = ( elBounds.x + docEl.scrollLeft - docEl.clientLeft ) * logicalZoomRatio;
y = ( elBounds.y + docEl.scrollTop - docEl.clientTop ) * logicalZoomRatio;
}
boxPos = 'absolute';
docEl = doc.documentElement;
x = ( elBounds.x + docEl.scrollLeft - docEl.clientLeft ) * logicalZoomRatio;
y = ( elBounds.y + docEl.scrollTop - docEl.clientTop ) * logicalZoomRatio;
}

s.position = boxPos;
s.left = x;
s.top = y;
s.zIndex = tgtPos === 'static' ? -1 : tgtCS.zIndex;
this.isPositioned = true;
boxPos = 'absolute';
}
},

updateVisibility: function() {
var vis = this.styleInfos.visibilityInfo,
box = this._box;
if ( box && vis.changed() ) {
vis = vis.getProps();
box.style.display = ( vis.visible && vis.displayed ) ? '' : 'none';
}
return 'direction:ltr;' +
'position:absolute;' +
'behavior:none !important;' +
'position:' + boxPos + ';' +
'left:' + x + 'px;' +
'top:' + y + 'px;' +
'z-index:' + ( tgtPos === 'static' ? -1 : tgtCS.zIndex ) + ';' +
'display:' + ( vis.visible && vis.displayed ? 'block' : 'none' );
},

updateRendering: function() {
if( this.isActive() ) {
this.updateVisibility();
} else {
this.destroy();
updateBoxStyles: function() {
var me = this,
boxEl = me.getBoxEl();
if( boxEl && ( me.boundsInfo.positionChanged() || me.styleInfos.visibilityInfo.changed() ) ) {
boxEl.style.cssText = me.getBoxCssText();
}
},

Expand All @@ -111,48 +91,59 @@ PIE.RootRenderer = PIE.RendererBase.newRenderer( {
/**
* Render any child rendrerer shapes which have not already been rendered into the DOM.
*/
finishUpdate: function() {
updateRendering: function() {
var me = this,
queue = me._shapeRenderQueue,
renderedShapes, markup, i, len, j,
ref, pos;

if( queue ) {
// We've already rendered something once, so do incremental insertion of new shapes
renderedShapes = me._renderedShapes;
if( renderedShapes ) {
for( i = 0, len = queue.length; i < len; i++ ) {
for( j = renderedShapes.length; j--; ) {
if( renderedShapes[ j ].ordinalGroup < queue[ i ].ordinalGroup ) {
break;
ref, pos, vis;

if (me.isActive()) {
if( queue ) {
// We've already rendered something once, so do incremental insertion of new shapes
renderedShapes = me._renderedShapes;
if( renderedShapes ) {
for( i = 0, len = queue.length; i < len; i++ ) {
for( j = renderedShapes.length; j--; ) {
if( renderedShapes[ j ].ordinalGroup < queue[ i ].ordinalGroup ) {
break;
}
}
}

if ( j < 0 ) {
ref = me.getBoxEl();
pos = 'afterBegin';
} else {
ref = renderedShapes[ j ].getShape();
pos = 'afterEnd';
if ( j < 0 ) {
ref = me.getBoxEl();
pos = 'afterBegin';
} else {
ref = renderedShapes[ j ].getShape();
pos = 'afterEnd';
}
ref.insertAdjacentHTML( pos, queue[ i ].getMarkup() );
renderedShapes.splice( j < 0 ? 0 : j, 0, queue[ i ] );
}
ref.insertAdjacentHTML( pos, queue[ i ].getMarkup() );
renderedShapes.splice( j < 0 ? 0 : j, 0, queue[ i ] );
}
}
// This is the first render, so build up a single markup string and insert it all at once
else {
queue.sort( me.shapeSorter );
markup = [ '<css3pie id="_pie' + PIE.Util.getUID( me ) + '" style="direction:ltr;position:absolute;">' ];
for( i = 0, len = queue.length; i < len; i++ ) {
markup.push( queue[ i ].getMarkup() );
me._shapeRenderQueue = 0;
me.updateBoxStyles();
}
markup.push( '</css3pie>' );
// This is the first render, so build up a single markup string and insert it all at once
else {
vis = me.styleInfos.visibilityInfo.getProps();
if( vis.visible && vis.displayed ) {
queue.sort( me.shapeSorter );
markup = [ '<css3pie id="_pie' + PIE.Util.getUID( me ) + '" style="' + me.getBoxCssText() + '">' ];
for( i = 0, len = queue.length; i < len; i++ ) {
markup.push( queue[ i ].getMarkup() );
}
markup.push( '</css3pie>' );

me.getPositioningElement().insertAdjacentHTML( 'beforeBegin', markup.join( '' ) );
me.getPositioningElement().insertAdjacentHTML( 'beforeBegin', markup.join( '' ) );

me._renderedShapes = queue;
me._renderedShapes = queue;
me._shapeRenderQueue = 0;
}
}
} else {
me.updateBoxStyles();
}
me._shapeRenderQueue = 0;
} else {
me.destroy();
}
},

Expand Down
Loading

12 comments on commit 220dc17

@BMCouto
Copy link

Choose a reason for hiding this comment

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

I still have problems with this... in my case i have an element with rounded corners in the middle of a text, and when changing that text the position of the element also changes but the shapes remain on the "old" position. Any idea of what may be causing this?

@lojjic
Copy link
Owner Author

@lojjic lojjic commented on 220dc17 Jul 11, 2012

Choose a reason for hiding this comment

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

@BMCouto You're using a master branch build? Any particular IE versions this issue occurs in?

@BMCouto
Copy link

Choose a reason for hiding this comment

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

I just made a build from your latest release here on github. It's happening on IE8.

@lojjic
Copy link
Owner Author

@lojjic lojjic commented on 220dc17 Jul 11, 2012

Choose a reason for hiding this comment

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

OK thanks, it's always great to see people trying master. I assume you've set -pie-load-path so it loads the js files from your server (this is not documented yet but is required for the time being.)

Would you be able to put together a testcase?

@BMCouto
Copy link

Choose a reason for hiding this comment

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

I didn't... where should I put this -pie-load-path and why?

@lojjic
Copy link
Owner Author

@lojjic lojjic commented on 220dc17 Jul 11, 2012

Choose a reason for hiding this comment

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

Because of this: aa82152 (-pie-base-url was renamed to -pie-load-path afterward)

Look at the html files in the test directory for an example of using -pie-load-path.

FYI this is only required right now until we have a real CDN default location.

@BMCouto
Copy link

Choose a reason for hiding this comment

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

I'm sorry but I'm not understanding. I never used -pie-load-path too and I can't find it on the html files. Can you give me an example and tell me why is it needed? I have a class for it which is used in several elements on the site.

@lojjic
Copy link
Owner Author

@lojjic lojjic commented on 220dc17 Jul 11, 2012

Choose a reason for hiding this comment

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

As described in that commit comment, -pie-load-path tells PIE.htc where to go to load its secondary .js files. Like I said, the docs in the tests directory are good examples of its usage, e.g. https://github.com/lojjic/PIE/blob/master/tests/basic.html

Since you weren't aware of these 2.x changes on master, may I ask why you are trying to use the master code? Is there a particular bug or feature you are trying to get around, or just for curiosity? I'm happy to have people testing master but if you want stable code for a live site the 1.0.0 release would be better for you.

@BMCouto
Copy link

Choose a reason for hiding this comment

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

Got it! :)
About using the master, I just prefer it because it has more bug fixes and performance improves. I come regularly to your github so I normally know what's new (although I missed the -pie-load-path)

@BMCouto
Copy link

Choose a reason for hiding this comment

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

Jason, I though the PIE.htc file was enough. In that case which files exactly do I need to have too?

@lojjic
Copy link
Owner Author

@lojjic lojjic commented on 220dc17 Jul 12, 2012

Choose a reason for hiding this comment

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

You need PIE.htc plus the two js files PIE_IE678.js and PIE_IE9.js -- it's easiest just to copy the entire build directory to your server.

@BMCouto
Copy link

Choose a reason for hiding this comment

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

Working fine, although I fixed with reposition the button again, because the initial problem continues. If possible you should check this out.
To reproduce it just put an element with rounded corners in a middle of a text and change that text after so the button moves his position after PIE rendering you will see the problem immediately.
Thanks a lot for all the help with -pie-load-path and keep on the amazing work done here! :)

Please sign in to comment.