Opened 11 years ago

Closed 8 years ago

#8867 closed bug (fixed)

Effects-Shake: Select element with padding breaks in 1.9.2

Reported by: abacada Owned by: mikesherov
Priority: minor Milestone: 1.12.0
Component: ui.effects.* (individual effect) Version: 1.9.2
Keywords: Cc:
Blocked by: Blocking:

Description

Using google chrome 23.0.1271.91

1 - Click the button. 2 - The shake effect occurs normally in the select element. http://jsfiddle.net/Jqpcw/2/ - 1.8.9

1 - Click the button. 2 - The shake effect reduces the original size of the select. 3 - Expected: No changes are made to the select element. http://jsfiddle.net/Jqpcw/1/ - 1.9.2

This is probably related to #5245. I can't figure if this is a jquery or jquery ui bug.

Change History (13)

comment:1 Changed 11 years ago by Scott González

Component: ui.coreui.effects.* (individual effect)
Status: newopen

comment:2 Changed 11 years ago by abacada

Any news on this one?

comment:3 Changed 11 years ago by abacada

For anyone interested I have made an workaround to this bug. It restricts the height/width change when using the "Shake" effect, kind of hacky but at least passes in all /effects/effects.html tests. Should be removed when fixed.

  if ( $.ui ) {
                (function() {
                        var oldEffect = $.fn.effect;
                        $.fn.effect = function( effectName ) {
                                if ( effectName === "shake" ) {
                                        var old = $.effects.createWrapper;
                                        $.effects.createWrapper = function( element ) {
                                                var result;
                                                var oldCSS = $.fn.css;
                                                
                                                $.fn.css = function( size ) {
                                                        var _element = this;
                                                        var hasOwn = Object.prototype.hasOwnProperty;
                                                        return _element === element
                                                                        && hasOwn.call( size, "width" )
                                                                        && hasOwn.call( size, "height" )
                                                                        && _element || oldCSS.apply( this, arguments );
                                                };
                                                
                                                result = old.apply( this, arguments );
                                                
                                                $.fn.css = oldCSS;
                                                return result;
                                        };
                                }
                        return oldEffect.apply( this, arguments );
                        };
                })();
        }

comment:4 Changed 10 years ago by mikesherov

Owner: set to mikesherov
Status: openassigned

This is most likely another side effect of the difference between .css("height") and .height() for border-box elements, and also a browser quirk in webkit regarding default margins for select elements. I'll investigate.

comment:5 Changed 10 years ago by mikesherov

this is effecting the demo for sure: https://github.com/necolas/normalize.css/issues/126

comment:6 Changed 10 years ago by abacada

But not as bad as the select element. Any news on it?

comment:7 Changed 10 years ago by mikesherov

Yes, good progress has been made on the effects rewrite and it comes down to a crazy bug I discovered in WebKit: https://bugs.webkit.org/show_bug.cgi?id=107380

comment:8 Changed 10 years ago by tj.vantoll

Milestone: 1.10.0none

comment:9 Changed 10 years ago by ckn

This seems to be somehow related to box-sizing, when I add box-sizing:content-box to your fiddle it works fine. http://jsfiddle.net/bkeva/

comment:10 Changed 10 years ago by abacada

Of course.

But box-sizing: border-box is the most practical use, although the spec does not consistently applies to every known element (as it should).

Let's say one want to apply 100% width to an input inside a relative container, the only way to use the minimun amount of code for such common behavior is to code with border-box, instead of calculating the border (which is a pain).

The following style should work for every jquery UI component:

  * {
    box-sizing: border-box;
  }

comment:11 Changed 10 years ago by feedbloo

Hey guys,

I am using $1.9.1 and $UI 1.10.2. I think I am facing the same problem.

When I click in a button to shake it, the effect() function creates a wrapper for the button, but the wrapper created is not inheriting in its width and height an eventually padding value from the button.

So, the wrapper element created should compute the parent padding values: padding-left + padding-right applied to the wrapper width and padding-top + padding-bottom applied to the wrapper height.

Something like this below should be add inside the createWrapper() function:

var

borders = parseFloat($wrappedElem.css('border-width').replace('px')) * 2,

horizPadding = parseFloat($wrappedElem.css('padding-left').replace('px')) + parseFloat($wrappedElem.css('padding-right').replace('px')) + borders,

vertPadding = parseFloat($wrappedElem.css('padding-top').replace('px')) + parseFloat($wrappedElem.css('padding-bottom').replace('px')) + borders;

$wrapper.css('height', $wrappedElem.height() + vertPadding);

$wrapper.css('width', $wrappedElem.width() + horizPadding);

I have changed the createWrapper() with that code, now things are up and running well.

Cheers,

Marcelo

Last edited 10 years ago by feedbloo (previous) (diff)

comment:12 Changed 9 years ago by mikesherov

Milestone: none1.12.0

comment:13 Changed 8 years ago by Mike Sherov

Resolution: fixed
Status: assignedclosed

Effects: Rewrite

  1. Introduces a set of helper methods to easily create and define new effects.
  2. Uses clip animations and placeholders instead of wrappers for clip effects.
  3. Ensures all animations are detectable as animated

Fixes #10599 Fixes #9477 Fixes #9257 Fixes #9066 Fixes #8867 Fixes #8671 Fixes #8505 Fixes #7885 Fixes #7041

Closes gh-1017

Changeset: b6bec797d6a8ef0b377a866c38c67e66a626b45f

Note: See TracTickets for help on using tickets.