wicket-jquery-ui icon indicating copy to clipboard operation
wicket-jquery-ui copied to clipboard

n-way connected Sortables are not possible

Open annnoo opened this issue 6 years ago • 13 comments

Hey,

at the moment you can only connect a single instance of Sortable with another. N-way connections are not possible.

annnoo avatar Jul 16 '18 15:07 annnoo

Hi,

Right, this is the current implementation. I will try to have a look over the week-end, but I guess the SortableBehavior#getConnectedList will cause problem...

Thanks & best regards, Sebastien.

sebfz1 avatar Jul 18 '18 09:07 sebfz1

Currently working on some kind of a reimplementation to get it working for 6.2. I don't see any big differences between the current version and 6.2 so I could maybe fork and implement it (if I can get it working).

annnoo avatar Jul 18 '18 11:07 annnoo

Please go ahead! :)

1/ The first update to perform is the ability to provide a common selector, I would have changed Sortable this way:

/**
 * Connects with another {@link Sortable}<br>
 * The specified {@link Sortable} will keep a reference to the caller ({@code this}).
 *
 * @param sortable the {@link Sortable} to connect with
 * @return this, for chaining
 */
public Sortable<T> connectWith(Sortable<T> sortable)
{
	return this.connectWith(sortable, JQueryWidget.getSelector(sortable));
}

/**
 * Connects with another {@link Sortable}<br>
 * The specified {@link Sortable} will keep a reference to the caller ({@code this}).
 *
 * @param sortable the {@link Sortable} to connect with
 * @param selector the html selector
 * @return this, for chaining
 */
public Sortable<T> connectWith(Sortable<T> sortable, String selector)
{
	Args.notNull(sortable, "sortable");

	sortable.connect(this); // eq. to sortable.connectedSortable = this;

	return this.connectWith(selector);
}

2/ The second update is to make a list of Sortable instead of a single connectedSortable and change #connect accordingly.

3/ SortableBehavior#getConnectedList should return the list of the actual connected sortable. I don't know yet how to identify it...

sebfz1 avatar Jul 18 '18 12:07 sebfz1

My current solution is somehow working but it is quite dirty...

1/ So instead of setting the JQuery option connectWith to an array of Sortable ids you would connect them via the same class?

3/ My current Solution is searching through all connected lists... not very performant, but it works:

 if (event instanceof ReceiveEvent) {
	List<List<T>> connectedLists = this.getConnectedLists();
	for (List<T> connected : connectedLists) {
		if (findItem(hash, connected) != null) {
			this.onReceive(target, findItem(hash, connected), index);
			break;
		}
	}
}

annnoo avatar Jul 18 '18 14:07 annnoo

I think we are on the same track 1/ yes, precisely. 3/ yes, something like this, you do not have much other choices. In another hand, you won't be connected to 10k sortables so... I had in mind a list of sortable that I don't read in your snippet, can you commit so I have a look ?

sebfz1 avatar Jul 18 '18 14:07 sebfz1

1/ Okay, currently I am doing it a bit different. I grab the id's of every connected Sortable (saved in a list) and set the option connectWith via Options.asString(List<String> values).

Will do a commit later this evening. :)

annnoo avatar Jul 18 '18 15:07 annnoo

Not sure this is supported by the plugin, it seems to accept only one selector. http://api.jqueryui.com/sortable/#option-connectWith But if it accepts several selectors, that's better of course.

sebfz1 avatar Jul 18 '18 15:07 sebfz1

Yeah it does. It supports a selector and this can be nearly anything :) See: http://api.jquery.com/category/selectors/

https://jsfiddle.net/evcy4qt1/5 You can either use and array of selectors (connectWith: "#sortable1","#sortable2","#sortable3") or a selector of multiple ids using a comma as delimiter (connectWith: ["#sortable1, #sortable2, #sortable3"])

annnoo avatar Jul 18 '18 15:07 annnoo

Perfect then! :)

sebfz1 avatar Jul 18 '18 15:07 sebfz1

commited it finally... but i am not sure if it works. Another thing i noticed is that i have to reformat the code manually, will do this today or tomorrow

annnoo avatar Jul 23 '18 08:07 annnoo

Looks like what we talked about ; seems fine at a first glance. Yes, please reformat the code, remove the system.out and I think you are good for the PR! :) Then I will do a proper review and merge the PR... Thanks!

sebfz1 avatar Jul 26 '18 15:07 sebfz1

Woops... Missed the sysout :D Standard Eclipse formatter or anything special?

annnoo avatar Jul 26 '18 18:07 annnoo

IIRC, i've a formatter at the project root...

sebfz1 avatar Jul 26 '18 19:07 sebfz1