sebfz1 / wicket-jquery-ui

jQuery UI & Kendo UI integration in Wicket
http://www.7thweb.net/wicket-jquery-ui/
Other
93 stars 58 forks source link

n-way connected Sortables are not possible #295

Open annnoo opened 6 years ago

annnoo commented 6 years ago

Hey,

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

sebfz1 commented 6 years ago

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.

annnoo commented 6 years ago

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).

sebfz1 commented 6 years ago

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...

annnoo commented 6 years ago

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;
        }
    }
}
sebfz1 commented 6 years ago

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 ?

annnoo commented 6 years ago

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 values).

Will do a commit later this evening. :)

sebfz1 commented 6 years ago

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.

annnoo commented 6 years ago

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"])

sebfz1 commented 6 years ago

Perfect then! :)

annnoo commented 6 years ago

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

sebfz1 commented 6 years ago

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!

annnoo commented 6 years ago

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

sebfz1 commented 6 years ago

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