prototypejs / prototype

Prototype JavaScript framework
http://prototypejs.org/
Other
3.54k stars 640 forks source link

clonePosition performance #231

Open jwestbrook opened 10 years ago

jwestbrook commented 10 years ago

previous lighthouse ticket #1328 by Victor


Code of Element#clonePosition is inefficient when invoked to clone only one of subset: left/top or width/height.

In first case - Element.clonePosition(element, source, {setWidth: false, setHeight: false}) - the code var layout = Element.getLayout(source); isn't needed.

In second case - Element.clonePosition(element, source, {setLeft: false, setTop: false}) - the code

var p = Element.viewportOffset(source), delta = [0, 0];

if (Element.getStyle(element, 'position') === 'absolute') {
  var parent = Element.getOffsetParent(element);
  if (parent !== document.body) delta = Element.viewportOffset(parent);
}

is unused, and just wasting time in Element.viewportOffset(), Element.getStyle(), Element.getOffsetParent().

The whole method could be rewritten as

 function clonePosition(element, source, options) {
    options = Object.extend({
      setLeft:    true,
      setTop:     true,
      setWidth:   true,
      setHeight:  true,
      offsetTop:  0,
      offsetLeft: 0
    }, options || {});

    source  = $(source);
    element = $(element);
    var p, delta, layout, styles = {};

    if (options.setLeft || options.setTop) {
      p = Element.viewportOffset(source);
      delta = [0, 0];
      if (Element.getStyle(element, 'position') === 'absolute') {
        var parent = Element.getOffsetParent(element);
        if (parent !== document.body) delta = Element.viewportOffset(parent);
      }
    }

    if (options.setWidth || options.setHeight) {
      layout = Element.getLayout(source);
    }

    if (options.setLeft)
      styles.left = (p[0] - delta[0] + options.offsetLeft) + 'px';
    if (options.setTop)
      styles.top  = (p[1] - delta[1] + options.offsetTop)  + 'px';

    if (options.setWidth)
      styles.width  = layout.get('border-box-width')  + 'px';
    if (options.setHeight)
      styles.height = layout.get('border-box-height') + 'px';

    return Element.setStyle(element, styles);
  }

or code could be moved to two simpler smaller methods, e.g. cloneSize() and cloneOffset(), and clonePosition() will just invoke these two methods (but this may cause additional reflows/repaints in browser).