Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#8377 closed bug (notabug)

Draggable documention should say that appendTo is for use with helper

Reported by: andrewbadr Owned by: andrewbadr
Priority: minor Milestone: 2.0.0
Component: ui.draggable Version: 1.8.20
Keywords: Cc:
Blocked by: Blocking:

Description

The documention for the appendTo option on draggable doesn't say that it only works if the helper element isn't already in the DOM. The relevant line is:

if(!helper.parents('body').length)
    helper.appendTo((o.appendTo == 'parent' ? this.element[0].parentNode : o.appendTo));

This seems to be meant only for use with the 'clone' option, but there's nothing saying that. It could be confusing.

Change History (9)

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

Summary: Draggable documention should say that appendTo is for use with 'clone'Draggable documention should say that appendTo is for use with helper

comment:2 Changed 8 years ago by Scott González

Owner: set to andrewbadr
Status: newpending

Actually, the document is pretty clear that it's for the helper. "The element passed to or selected by the appendTo option will be used as the draggable helper's container during dragging. By default, the helper is appended to the same container as the draggable." How would you word this differently so as to be explicit but not redundant?

comment:3 Changed 8 years ago by andrewbadr

Status: pendingnew

What does "helper" mean?

  1. The element that is being dragged, regardless of whether it is the original draggable or a cloned element, OR
  2. Only the "clone" case

Your changes and comment on this ticket, if I understand them correctly, imply that definition 2 is correct. If so, that's the source of my misunderstanding. But the docs are far from clear, in ways that I can elaborate on.

Last edited 8 years ago by andrewbadr (previous) (diff)

comment:4 Changed 8 years ago by Scott González

Status: newpending

#2, but your definition is wrong. Cloning is one of an infinite number of possibilities for a helper. The documentation and default value for helper are confusing, because helper: "original" is really "no helper". This is being addressed in the API redesign in which the default will be helper: null.

Please provide your suggestions for updates to make the documentation more clear.

comment:5 Changed 8 years ago by andrewbadr

Status: pendingnew

The terms should be defined somewhere, probably on the overview page.

Besides the 'helper' option documentation that you already mentioned, the main problem is the naming on the callback argument, ui.helper. If no helper is specified, ui.helper refers to the original draggable, strongly suggesting that definition (1) above is correct.

comment:6 Changed 7 years ago by Scott González

Milestone: 1.9.02.0.0

comment:7 Changed 7 years ago by mikesherov

Keywords: needsdocs added
Resolution: notabug
Status: newclosed

Thanks andrewbadr! Im closing this temporarily to eventually move it to the API docs issue tracker.

comment:8 Changed 7 years ago by Scott González

Keywords: needsdocs removed

Definition (1) above is not correct. The name of the ui property is what's confusing. If you have a suggestion for better wording that doesn't make the assumption that (1) applies to appendTo, please reply to this ticket or send a pull request to modify https://github.com/jquery/api.jqueryui.com/blob/master/entries/draggable.xml

Thanks.

comment:9 Changed 7 years ago by mikesherov

Thanks scott. I was only adding needsdocs as a temporary thing until I can in one shot port them over. Glad you took care of it.

Note: See TracTickets for help on using tickets.