search

chnm / anthologize

Anthologize is a free, open-source, WordPress-based platform for publishing. Grab posts from your WordPress blog, pull in feeds from external sites, or create new content directly in Anthologize. Then outline, order, and edit your work, crafting it into a coherent volume for export in several formats, including PDF, EPUB, and TEI.
http://anthologize.org
GNU General Public License v3.0
174 stars 28 forks source link

Draggable fromNew breaks in WP 3.3 #52

Closed boonebgorges closed 13 years ago

boonebgorges commented 13 years ago

It's not possible to add a new item to a project when using the latest WP 3.3 trunk version. It's probably a result of their switch to the latest jQuery.

I did a bit of debugging, and found that the newly dragged item isn't being given the proper id, so that when the ajax request is sent, there's no information about the post_id, and as a result nothing can be copied. The result is an AJAX error and a forced refresh.

I'll do some more debugging when I can, but it'd be nice if someone else more familiar with the JS could help :)

patrickmj commented 13 years ago

I'll do some digging to see what I can come up with.

cazzerson commented 13 years ago

It looks like this change is the cause of the problem:

http://bugs.jqueryui.com/ticket/4564

and is related to cloning the original element for the drag/copy:

http://jqueryui.com/demos/draggable/#option-helper

We were basically expecting duplicate IDs before. We'll have to look at this a bit more, but it seems like a couple possible solutions are:

1) Grab the ID of the original item as the dragging is about to start. 2) Store the post_id in some other attribute, such as the class, or in a nested hidden span.

The second one might be a little more proper. Any thoughts?

Jason

On Wed, Nov 16, 2011 at 12:48 PM, Boone Gorges reply@reply.github.com wrote:

It's not possible to add a new item to a project when using the latest WP 3.3 trunk version. It's probably a result of their switch to the latest jQuery.

I did a bit of debugging, and found that the newly dragged item isn't being given the proper id, so that when the ajax request is sent, there's no information about the post_id, and as a result nothing can be copied. The result is an AJAX error and a forced refresh.

I'll do some more debugging when I can, but it'd be nice if someone else more familiar with the JS could help :)

Reply to this email directly or view it on GitHub: https://github.com/chnm/anthologize/issues/52

boonebgorges commented 13 years ago

Good find, Jason!

Something along the second lines sounds good to me. If it the post_id can be stowed in another class, it seems like it would introduce the least overhead.

On 11/16/11 3:58 PM, Jason Casden wrote:

It looks like this change is the cause of the problem:

http://bugs.jqueryui.com/ticket/4564

and is related to cloning the original element for the drag/copy:

http://jqueryui.com/demos/draggable/#option-helper

We were basically expecting duplicate IDs before. We'll have to look at this a bit more, but it seems like a couple possible solutions are:

1) Grab the ID of the original item as the dragging is about to start. 2) Store the post_id in some other attribute, such as the class, or in a nested hidden span.

The second one might be a little more proper. Any thoughts?

Jason

On Wed, Nov 16, 2011 at 12:48 PM, Boone Gorges reply@reply.github.com wrote:

It's not possible to add a new item to a project when using the latest WP 3.3 trunk version. It's probably a result of their switch to the latest jQuery.

I did a bit of debugging, and found that the newly dragged item isn't being given the proper id, so that when the ajax request is sent, there's no information about the post_id, and as a result nothing can be copied. The result is an AJAX error and a forced refresh.

I'll do some more debugging when I can, but it'd be nice if someone else more familiar with the JS could help :)

Reply to this email directly or view it on GitHub: https://github.com/chnm/anthologize/issues/52

Reply to this email directly or view it on GitHub: https://github.com/chnm/anthologize/issues/52#issuecomment-2767799

patrickmj commented 13 years ago

I'd lean toward the nested hidden span over a class on the element, just since it seems like non-classy data. But then again, I've never been overly concerned with being classy.

boonebgorges commented 13 years ago

We're a classless bunch, aren't we?

On 11/16/11 4:58 PM, Patrick Murray-John wrote:

I'd lean toward the nested hidden span over a class on the element, just since it seems like non-classy data. But then again, I've never been overly concerned with being classy.

Reply to this email directly or view it on GitHub: https://github.com/chnm/anthologize/issues/52#issuecomment-2768656

cazzerson commented 13 years ago

OK, I think I've got a working fix, but I haven't tested it in WP 3.2 and I would also appreciate another check in WP 3.3. Do we have a procedure for testing changes? I could push a branch, or just push the changes to master and pull them back if needed, or I could send you all a patch.

boonebgorges commented 13 years ago

If it's just a single changeset/commit, just push it to the master branch. We can revert if it's broken. If it's multiple changesets, push up a separate branch so that things don't get too messy.

On 11/17/11 11:12 AM, Jason Casden wrote:

OK, I think I've got a working fix, but I haven't tested it in WP 3.2 and I would also appreciate another check in WP 3.3. Do we have a procedure for testing changes? I could push a branch, or just push the changes to master and pull them back if needed, or I could send you all a patch.

Reply to this email directly or view it on GitHub: https://github.com/chnm/anthologize/issues/52#issuecomment-2777864

patrickmj commented 13 years ago

I'm happy with a push to master -- I think we had an awful lot of branch proliferation for a while!

On 11/17/2011 11:12 AM, Jason Casden wrote:

OK, I think I've got a working fix, but I haven't tested it in WP 3.2 and I would also appreciate another check in WP 3.3. Do we have a procedure for testing changes? I could push a branch, or just push the changes to master and pull them back if needed, or I could send you all a patch.

Reply to this email directly or view it on GitHub: https://github.com/chnm/anthologize/issues/52#issuecomment-2777864

boonebgorges commented 13 years ago

Branch "proliferation" is one of the selling points of Git as a vcs :)

On 11/17/11 11:15 AM, Patrick Murray-John wrote:

I'm happy with a push to master -- I think we had an awful lot of branch proliferation for a while!

On 11/17/2011 11:12 AM, Jason Casden wrote:

OK, I think I've got a working fix, but I haven't tested it in WP 3.2 and I would also appreciate another check in WP 3.3. Do we have a procedure for testing changes? I could push a branch, or just push the changes to master and pull them back if needed, or I could send you all a patch.

Reply to this email directly or view it on GitHub:

https://github.com/chnm/anthologize/issues/52#issuecomment-2777864

Reply to this email directly or view it on GitHub: https://github.com/chnm/anthologize/issues/52#issuecomment-2777926

cazzerson commented 13 years ago

I'm still getting used to proliferating branches. Anyways, changes pushed to master.

boonebgorges commented 13 years ago

Sweet - just got a chance to look at this. Looks great on WP 3.3 and 3.2.x. I moved your fix over to the 0.6.x branch too.

Thanks!