Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: put: false not working #1416

Closed
johanneswilm opened this issue Jan 7, 2019 · 11 comments
Closed

Regression: put: false not working #1416

johanneswilm opened this issue Jan 7, 2019 · 11 comments

Comments

@johanneswilm
Copy link

Problem:

put: false is not working in 1.8.0-rc1. It is working in 1.7.0.

Please let me know if the issue is not immediately obvious, in which case I will try to create a jsfiddle.

@owen-m1
Copy link
Member

owen-m1 commented Jan 7, 2019

@johanneswilm Please ensure the two Sortables you are using do not have the same group name, otherwise the put option will be ignored

@johanneswilm
Copy link
Author

@owen-m1 ok, then maybe I don't understand how this is supposed to work in 1.8.0. As I understand it, and as it seems to be working in 1.7.0 and what the documentation seems to say, the group name makes sure that you can move between lists using the same group name. So say I have two lists and I only want items from list one to be movable to list two, but not the other way round, I do:

List one:

group: {
  name: 'myitems',
  pull: true,
  put: false
}

List two:

group: {
  name: 'myitems',
  pull: false,
  put: true
}

Did I misunderstand the documentation and is this just accidentally working for me in 1.7.0? If so, how would I achieve the same goal?

@johanneswilm
Copy link
Author

I kind of need to do what is being done here:

Sortable/st/app.js

Lines 119 to 139 in 96a5fec

// Advanced groups
[{
name: 'advanced',
pull: true,
put: true
},
{
name: 'advanced',
pull: 'clone',
put: false
}, {
name: 'advanced',
pull: false,
put: true
}].forEach(function (groupOpts, i) {
Sortable.create(byId('advanced-' + (i + 1)), {
sort: (i != 1),
group: groupOpts,
animation: 150
});
});

If I'm not mistaken, the group name stays the same in all those three, while pull/put are being changed, right?

@owen-m1
Copy link
Member

owen-m1 commented Jan 8, 2019

I am not entirely sure of the original intent, but I know for a fact that it was broken before. I believe the current functionality was the original intent. If you want the pull and put to be applied, you can either give them different group names or you can remove the group name entirely.

The reason I believe that this was the original intent is because there is the option of setting pull or put to an array of group names that it should allow. But I am a new maintainer so not 100% sure.

Also, pull: "clone" should still apply, even for two Sortables in the same group.

@johanneswilm
Copy link
Author

I don't think your change makes sense. Say you have several lists with the same group. group.name is the same and say group.put === false. So then you cannot move anything between those lists, so what's the point of having the same group? The consequence of your change is then that put and pull should never be false, right?

Could we ask a previous maintainer about this? I think this is a quite central feature of 1.7.0 so unless it's restored as it was or the functionality is reimplemented in some other way, we are missing something here. I just need to be able to do what is happening in the demo I linked to in my last reply here.

@owen-m1
Copy link
Member

owen-m1 commented Jan 8, 2019

No, you are wrong about the behavior. In the case that you have several sortables with the same group name, put can be false, but you can still put items from sortables with the same group name into it. You just cannot put items from another group in it (therefore the default value for put is false).

The way I see it, if it was set up so that pull and put apply to the same group, then the entire idea of having groups would be redundant, because pull and put would be allowing or denying both items from the same group and items from a different groups.

Perhaps it can be such that if a sortable has the same group as another sortable, the default will be that pull and put are both true between them, but if a different value is given by the user, it will be taken into account. Whereas if it were a different group the default is that both of these will be false.

@johanneswilm
Copy link
Author

johanneswilm commented Jan 8, 2019

No, you are wrong about the behavior.

Have you looked at the example I linked to above? That is part of the demo of this repo and afaict it works as I described.

I don't think it makes groups redundant either. Groups can be things like "people", "books" and "cars" that all need to be on the same webpage (for some reason) but shouldn't be draggable into oneanother's containers. Having the group name disable the other option does not seem to be helpful.

As for the last paragraph: I am not sure I understood it fully, but basically you seem to be saying you are willing to reestablish the previous behavior as a config option. I'm fine with such a solution.

@owen-m1
Copy link
Member

owen-m1 commented Jan 8, 2019

@johanneswilm Yes, you are right, the demo does indicate that the behavior you described would be what is intended. And yes, I believe this would be covered by the change I described in the last paragraph of my previous comment. I will include this fix in 1.8.0. For now, use 1.7.0 if you need this :).

@johanneswilm
Copy link
Author

Great. Thank you!

@tedwardmah
Copy link

👍 the functionality described by @johanneswilm is crucial for our use case as well (which we developed using the approaches from the demos)

@owen-m1
Copy link
Member

owen-m1 commented Jan 15, 2019

@johanneswilm @tedwardmah 1.8.0 is now released, with the desired behavior

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants