Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

fx_methods show() and hide() add unnecessary CSS properties on non-animated DOM nodes #445

Open
pamelafox opened this Issue · 5 comments

5 participants

@pamelafox

When you add fx_methods to your Zepto, the show() and hide() methods are overriden with methods that also call animate. Even in the case of undefined speed (normal show), animate still adds many CSS properties to the shown/hidden notes, e.g.:
display: block; -webkit-transform-origin-x: 0px; -webkit-transform-origin-y: 0px; opacity: 1; -webkit-transform: scale3d(1, 1, 1); -webkit-animation-name: none; -webkit-transition-property: none; -webkit-transition-duration: initial; -webkit-transition-timing-function: initial; -webkit-transition-delay: initial;

These unexpected CSS rules can interfere with the page CSS (in my case, it has changed the positioning of my child fixed DIVs).

The equivalent jQuery methods do not call animate in the case of undefined speed, so I am currently experimenting with this change to the code:

  $.fn.show = function(speed, callback) {
    if (speed || speed === 0) {
      this.css('opacity', 0);
      return anim(this, speed, 1, '1,1', callback);
    } else {
      return origShow.call(this);
    }
  };

  $.fn.hide = function(speed, callback) {
    if (speed || speed === 0) {
      return hide(this, speed, '0,0', callback);
    } else {
      return origHide.call(this);
    }
  }
@mislav
Collaborator

I agree that, if there's no animation, there should be no changes to styles of the element.

@pamelafox

Also: The current fx_methods.js show/hide code changes the opacity in the case of a normal show() - I had a div with opacity set at .7, and it was re-set to 1.0 after the show().

I don't think my code is quite correct though.

@Grsmto

Up for this. Zepto should not change anything else than display property. At the moment it breaks all css transitions applied to the element you want to show/hide.

@acemir

Up for this. (2)

Same thing here! The 'transform' property realy cancels the 'fixed' positioned child elements, and for the 'absolute' positioned child elements, it acts like a 'relative' positioned element, preventing them to position themselves relative to their parent elements.

My suggested solution is this:

function hide(el, speed, scale, callback) {
  return anim(el, speed, 0, scale, function(){
    origHide.call($(this))
    callback && callback.call(this)
  })
}

function show(el, speed, scale, callback) {
  origShow.call(el)
  return anim(el, speed, 1, scale, callback)
}

$.fn.show = function(speed, callback) {
  return (speed === undefined) ? origShow.call(this) : show(this, speed, '1,1', callback)
}

$.fn.hide = function(speed, callback) {
  return (speed === undefined) ? origHide.call(this) : hide(this, speed, '0,0', callback)
}

Thus, by calling methods show() or hide() without the speed parameter setted, we exchanged only the 'display' property, and when the parameter is set, the event continues to be called the classic effect of element scale.

@acemir acemir referenced this issue from a commit in acemir/zepto
@acemir acemir Fix for #445
Fix for #445
Commente here: madrobby#445 (comment)
acbc9d7
@madrobby
Owner

This breaks show with no speed given after a hide with an animation speed.

Perhaps the code could temporarily store the original opacity value before an animated hide, so that show can apply the correct value if no speed is given?

Any takers to implement this? If so, a unit test for fx_methods would be most welcome.

@madrobby madrobby was assigned
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.