animateClass in dev-branch: Names for the new options and code review

[alexander farkas]

Hello,

the animateClass Plugin is quite final now. The only things to do is
naming, documentation of the new features and a little code review by
a third UI developer.

I introduced the following new options:

'clearInlineStyles' [boolean: false]: this option ensures, that
currently bound inline-styles won´t override any (animation) styles
defined in an external stylesheet. (technically: all inline styles
will be deleted before the newStyle-object is collected.)

'animateChilds': allows you to reference descendant-elements, wich
should be also animated

'filterCallback' [function]: this is an advanced, but very usefull
feature. it allows the author to manipulate the animateStyles-object
and resetStyle-string before these are passed to the jQuery´s animate-
method. This is usefull, if you wan´t to animate an element, wich has
also dynamically added inline styles, wich can´t be referenced in a
stylesheet. (for example you are positioning the top/left values of an
alert-box with JavaScript and then want to animate it with
animateClass.)

The this keyword in the filterCallback points to the DOM-object and
the filterCallback is passed one argument with an object, that has the
following propterties:

animateStyles: object with all Styles, wich should be animated,

resetStyle: string with the styles, that should be attached when the
animation is finished (normally an empty string (whitespace for IE8))

as an addition there are three extra properties:

element: an jQuery-Object,
newStyle: an object with all collected styles,
oldStyle: an object with all collected styles

the difference between newStyle and oldStyle is the basis of the
animateStyles-object.

you can find a demo/visual test @
http://jquery-ui.googlecode.com/svn/branches/dev/effects/tests/visual/animateClass/

The following tickets are fixed in this version: http://dev.jqueryui.com/ticket/3939,
http://dev.jqueryui.com/ticket/3938, http://dev.jqueryui.com/ticket/3937,
http://dev.jqueryui.com/ticket/3112

As I´m very new to jQuery UI Dev it would be usefull, if someone could
make a code review before merging to trunk.

regards
alex

[polyrhythmic]

A couple quick notes on the code:
Lines 221 & 223, "deleteStyes" should be "deleteStyles" .

The leading semicolon on line 10 plays havoc with IE7 on Vista,
perhaps because of the script debugger. Random parts of my scripts
fail when I leave in the semicolon. I'm not sure why it's there...?

Thanks,

Charles


On Mar 22, 8:42 am, alexander farkas <a.farkas...@googlemail.com>
wrote:

> you can find a demo/visual test @http://jquery-ui.googlecode.com/svn/branches/dev/effects/tests/visual...
>
> The following tickets are fixed in this version:http://dev.jqueryui.com/ticket/3939,http://dev.jqueryui.com/ticket/3938,http://dev.jqueryui.com/ticket/3937,http://dev.jqueryui.com/ticket/3112

[Paul Bakaus]

2009/3/22 alexander farkas <a.farkas.pm@googlemail.com>

Hello,

the animateClass Plugin is quite final now. The only things to do is
naming, documentation of the new features and a little code review by
a third UI developer.

I introduced the following new options:

'clearInlineStyles' [boolean: false]: this option ensures, that
currently bound inline-styles won´t override any (animation) styles
defined in an external stylesheet. (technically: all inline styles
will be deleted before the newStyle-object is collected.)

While technologically nice, I don't think we should handle this case
through an option, but rather teach people to not use inline styles
themselves if they use animateClass.
 

'animateChilds': allows you to reference descendant-elements, wich
should be also animated

Actually I think this should not be an option, but the default. I see a slight
overhead arriving, but the correct behaviour clearly would be to check if
any of the children has changed styles, and animate them along. If we
keep the option, I still strongly suggest to have it be true by default.
 

'filterCallback' [function]: this is an advanced, but very usefull
feature. it allows the author to manipulate the animateStyles-object
and resetStyle-string before these are passed to the jQuery´s animate-
method. This is usefull, if you wan´t to animate an element, wich has
also dynamically added inline styles, wich can´t be referenced in a
stylesheet. (for example you are positioning the top/left values of an
alert-box with  JavaScript and then want to animate it with
animateClass.)

I really don't think we should expose animateClass that far. In your
specific example, the same result can simply be achieved by using
a class name that doesn't modify top/left values.

If there's one thing we're good at, then it's keeping things simple. All
options should always stay optional, and less options are always preferable,
if there is a actual workaround.
 

The this keyword in the filterCallback points to the DOM-object and
the filterCallback is passed one argument with an object, that has the
following propterties:

animateStyles: object with all Styles, wich should be animated,

resetStyle: string with the styles, that should be attached when the
animation is finished (normally an empty string (whitespace for IE8))

as an addition there are three extra properties:

element: an jQuery-Object,
newStyle: an object with all collected styles,
oldStyle: an object with all collected styles

the difference between newStyle and oldStyle is the basis of the
animateStyles-object.

you can find a demo/visual test @
http://jquery-ui.googlecode.com/svn/branches/dev/effects/tests/visual/animateClass/

The following tickets are fixed in this version: http://dev.jqueryui.com/ticket/3939,
http://dev.jqueryui.com/ticket/3938, http://dev.jqueryui.com/ticket/3937,
http://dev.jqueryui.com/ticket/3112

As I´m very new to jQuery UI Dev it would be usefull, if someone could
make a code review before merging to trunk.

My connection is broken right now, (love gmail offline..), but I'll check the code as
soon as I'm connected again.

Thanks for all your hard work on this!

Cheers,
Paul
 

regards
alex

--
Paul Bakaus
UI Architect
--
http://paulbakaus.com
http://www.linkedin.com/in/paulbakaus

[Todd Parker]

On Apr 7, 12:34 pm, Paul Bakaus <paul.bak...@googlemail.com> wrote:
> 2009/3/22 alexander farkas <a.farkas...@googlemail.com>

>
>
>
> > Hello,
>
> > the animateClass Plugin is quite final now. The only things to do is
> > naming, documentation of the new features and a little code review by
> > a third UI developer.
>
> > I introduced the following new options:
>
> > 'clearInlineStyles' [boolean: false]: this option ensures, that
> > currently bound inline-styles won´t override any (animation) styles
> > defined in an external stylesheet. (technically: all inline styles
> > will be deleted before the newStyle-object is collected.)
>
> While technologically nice, I don't think we should handle this case
> through an option, but rather teach people to not use inline styles
> themselves if they use animateClass.
>
>

Although I completely agree that we strongly discourage using inline
styles, the reality is that anything positioned (via positionTo) like
a dialog or menu will have inline styles for positioning. Animations
also tend to place inline styles on elements. For these cases, we need
to handle inline styles gracefully.

>
> > 'animateChilds': allows you to reference descendant-elements, wich
> > should be also animated
>
> Actually I think this should not be an option, but the default. I see a
> slight
> overhead arriving, but the correct behaviour clearly would be to check if
> any of the children has changed styles, and animate them along. If we
> keep the option, I still strongly suggest to have it be true by default.
>
>

+1 It would be nice is this automatically animated children so the
transition looks smooth.

> >http://jquery-ui.googlecode.com/svn/branches/dev/effects/tests/visual...

>
> > The following tickets are fixed in this version:
> >http://dev.jqueryui.com/ticket/3939,

> >http://dev.jqueryui.com/ticket/3938,http://dev.jqueryui.com/ticket/3937,

[Paul Bakaus]

On Tue, Apr 7, 2009 at 6:53 PM, Todd Parker <fg....@gmail.com> wrote:

On Apr 7, 12:34 pm, Paul Bakaus <paul.bak...@googlemail.com> wrote:
> 2009/3/22 alexander farkas <a.farkas...@googlemail.com>

>
>
>
> > Hello,
>
> > the animateClass Plugin is quite final now. The only things to do is
> > naming, documentation of the new features and a little code review by
> > a third UI developer.
>
> > I introduced the following new options:
>
> > 'clearInlineStyles' [boolean: false]: this option ensures, that
> > currently bound inline-styles won´t override any (animation) styles
> > defined in an external stylesheet. (technically: all inline styles
> > will be deleted before the newStyle-object is collected.)
>
> While technologically nice, I don't think we should handle this case
> through an option, but rather teach people to not use inline styles
> themselves if they use animateClass.
>
>

Although I completely agree that we strongly discourage using inline
styles, the reality is that anything positioned (via positionTo) like
a dialog or menu will have inline styles for positioning. Animations
also tend to place inline styles on elements. For these cases, we need
to handle inline styles gracefully.

I agree, but I still don't see a valid usecase that doesn't have a workaround.
Maybe you or Alexander have an idea of a possible setup?