search

One-com / knockout-dragdrop

A drag and drop binding for Knockout.
BSD 3-Clause "New" or "Revised" License
85 stars 29 forks source link

Feature/f 1 n target support #11

Closed BigDubb closed 10 years ago

BigDubb commented 10 years ago

I needed ability to have the target be different than the source for drop elements.

I'm not a CSS wizard, so I chose not to modify the CSS and rather just extend it in a new file. Im sure this can be cleaned up significantly.

Last commit feels jenky, might be a better way to handle this.

The example js was the out put of a Typescript compile, I just used this to make sure types were being set properly.

BigDubb commented 10 years ago

Why isn't this getting any feedback?

sunesimonsen commented 10 years ago

Sorry I'll take a look at it later today.

sunesimonsen commented 10 years ago

Sorry that you didn't get any feedback earlier. I'm also sorry but the quality of this pull request is not something I'm considering to merge, It includes weird files that shouldn't be part of the repository F-1 Description.txt, a new example that is not part of the main example like all the others and API changes that has not been discussed in an issue in advance. It is not to impolite but merging this would be too much work on my part without really seeing the gain.

I guest this is the issue you are trying to address: There are instances where the target may be dynamic. The tool should not assume the target and the source both reside in the same object.. The binding actually doesn't assume anything about sources or targets it just set the data of the drag zone to the current view model and the same goes for the drop zone. That might be a problem because usually you are looping though items and the binding is bound to the view models of the items instead of something else. That is definitely a problem I would consider to solve by adding a data option to the binding of the drag zone.

The way we are using the binding at One is to set a variable from the dragStart callback and use that when dropping. That works well but I agree that using a data option would be more elegant.

Sorry that I can't be more helpful Kind regards Sune Simonsen

sunesimonsen commented 10 years ago

I created an issue for the problem, please consider to comment: https://github.com/One-com/knockout-dragdrop/issues/14

BigDubb commented 10 years ago

Thanks for the feedback. I didn't have a good medium to communicate the problem, which is why I included that file. I thought about tying to retrofit my example in, but it was a bit more complicated in its implementation than the examples that were given. However, I can see why you don't want to merge it.

If you look through the example you can see how it created drop zones dynamically using templates so you have to rely on some type of convention to allow for appropriate drops etc.

I don't the solution you provided will work unless you have some type of dependency injection to know parents sources etc. I tried digging into what was being passed around and even while setting that It still lost context.

sunesimonsen commented 10 years ago

Please see issue #14 I created for this use case.