See my inline comments > On 30.04.2015, at 04:29, Maxim Solodovnik <solomax...@gmail.com> wrote: > > > The changes I have made were (most probably) merged from original Igor's > code (https://github.com/ivaynberg/wicket-select2)
I can’t find the "List<T> choices“ property of AbstractSelect2Choice anywhere in Igor’s original code, so I don’t know where it should have been merged from. What should be the purpose of having to provide a ChoiceProvider *and* a list of choices?! Currently you have to provide both to avoid an exception upon construction, which is … weird ;) and breaks existing implementations. > 1) I believe this is since AbstractSelect2Choice is parent class for for > Single and Multi choices. That is so, but what does this explain? ;) > 2) the code was refactored a bit, so line numbers were changed. This > constructor: > https://github.com/wicketstuff/core/blob/wicket-6.x/jdk-1.6-parent/select2-parent/select2/src/main/java/org/wicketstuff/select2/AbstractSelect2Choice.java#L109 > seems to not throwing any exceptions Yeah, but another does. This pattern does not make any sense to me: class Foo { public Foo(Object a) { this(a, null); } public Foo(Object a, Object b) { if (b == null) throw new IllegalArgumentException(); this.a = a; this.b = b; } } > 3) you can call constructor mentioned in 2 to avoid exception. … > 4) I guess renderChoice method is designed to render single choice object, > you can override it (for ex. to add your own properties to each choice) > other 2 methods are private helpers renderChoice is called from within renderChoices which is called from within renderHead. But only if !isAjax(), and: public boolean isAjax() { return provider != null; } …? That means: if no ChoiceProvider is set, then use the static List<T> choices, but if you don’t provide a ChoiceProvider in the constructor you’ll get an IllegalArgumentException. This code is broken and doesn’t make any sense *to me*, or I didn’t get it yet ;-) My proposal is: if nobody can tell which problem should be solved with the introduced List<T> choices I would revert it to get a working implementation again, as it is currently unusable. Personally, I need to upgrade the select.js library in our project, therefor I was looking at wicketstuff-select2, as we still use the original wicket-select2 implementation. The current state of it leaves mit three options, in order of priority: a) convince the wicketstuff-select2 maintainers to revert the code to a working state, which I am currently doing ;-) b) fork wicketstuff-select2 and provide own (working) implementation c) stick with Igor’s original code, which means: no upgrades to select2.js Maybe someone else can shed some light on this? Cheers, -Tom --------------------------------------------------------------------- To unsubscribe, e-mail: users-unsubscr...@wicket.apache.org For additional commands, e-mail: users-h...@wicket.apache.org
