Ticket #7106 (closed bug: fixed)
animateClass: css and class changes during animation are lost
| Reported by: | ddstreet | Owned by: | |
|---|---|---|---|
| Priority: | minor | Milestone: | 1.9.0 |
| Component: | ui.effects.core | Version: | 1.8.10 |
| Keywords: | Cc: | ||
| Blocking: | Blocked by: |
Description
When performing animation using animateClass (e.g. addClass, removeClass, etc. with a duration), any css changes to the animated elements done during animation are overwritten at the end of the animation, while I believe only css properties that are involved in the animation should be overwritten at the end of the animation. For example, the div background should not be reset to red at the end of the animation: http://jsbin.com/afele3
Change History
comment:3 Changed 19 months ago by ddstreet
Please update this bug's title to: animateClass: css and class changed during animation is overwritten and update the version to git.
The problem is now two-fold with the latest git code; not only are changes to the css lost at the end of animateClass(), but now changes to the class are also lost. Please see: http://jsbin.com/afele3/4
The class animation moves the box from left to right; during the animation the css is updated to change the background color, and a class is added to change the border; both of these changes are lost after the animation completes.
comment:4 Changed 19 months ago by scott.gonzalez
@ddstreet: version: git means that this bug has not been released.
comment:5 Changed 19 months ago by ddstreet
@scott.gonzalez: animateClass is broken is a worse way in the latest released code - elements with no initial class aren't animated at all, because .attr("class") returns null. Anyway, I'd like to track fixing the code in git, so should I just open a new bug and forget this one?
comment:6 Changed 19 months ago by scott.gonzalez
Then you should file a separate bug for the newly broken behavior.
comment:7 follow-up: ↓ 9 Changed 19 months ago by gnarf
I've actually already encountered this new bug (elements without a class) and fixed it: https://github.com/jquery/jquery-ui/commit/e8ba367a58995b191706e5682eacba534cd25697
Were you testing against the code in git, or the most recent 1.9 milestone release?
comment:8 Changed 19 months ago by gnarf
- Summary changed from animateClass: css changed during animation is overwritten to animateClass: css and class changes during animation are lost
comment:9 in reply to: ↑ 7 Changed 19 months ago by ddstreet
Replying to gnarf:
I've actually already encountered this new bug (elements without a class) and fixed it: https://github.com/jquery/jquery-ui/commit/e8ba367a58995b191706e5682eacba534cd25697
Were you testing against the code in git, or the most recent 1.9 milestone release?
The latest jsbin example: http://jsbin.com/afele3/4 is using the git code (specifically, the git effects.core.js, but 1.8.16 release for the rest); I saw in git that the class-less element bug was fixed, and so did not bother with a new bug for that (since it was already fixed in git).
Figured that it would just be easier (and include history) to update this bug to indicate the problems in git, but if I really should open a new bug for the git problems, let me know and I can do that...
comment:10 Changed 19 months ago by gnarf
Yeah, I updated this bug to include classes changed during the animation as well. The reasoning behind both css and class changes being broken is quite similar, so it will probably be easiest to solve them both at the same time.
Thanks for the update, I'll see if I can't take a closer look at providing a fix for this soon.
comment:11 Changed 19 months ago by ddstreet
I think this commit fixes both these issues; it also adds a test case for both issues. https://github.com/ddstreet/jquery-ui/commit/2510b74541dbf7ce19f0493e7ba5f86b669523ae
I submitted a pull request also https://github.com/jquery/jquery-ui/pull/503
It works in my limited testing. If the commit isn't acceptable, hopefully it at least helps with a starting point for the fix.
Thanks!
comment:12 Changed 19 months ago by ddstreet
- Status changed from open to closed
- Resolution set to fixed
Effects Core: Do not overwrite css or class changes that aren't animated during class animation. Fixed #7106 - animateClass: css and class changes during animation are lost
Changeset: e3156ea28617e6cc30a3389ee8d3f30514b9d894
comment:13 Changed 7 months ago by mikesherov
#8588 is a duplicate of this ticket.


https://gist.github.com/866242
This change (first patch) works for me with Chrome and Firefox (both latest), although I'm not completely sure if the .style var is usable as an object across all browsers...is there a better cross-browser way to do it?
Also, I added a test case for this, the second patch in the gist. I did add a new file as part of the patch, core_tickets.js