Lior Vernia has posted comments on this change.
Change subject: userportal,webadmin: type ahead list box
......................................................................
Patch Set 8: (3 inline comments)
In my opinion, the design here might be flawed. It doesn't make sense for a
ListModelSuggestBox to be generic, and the implementation of
BaseListModelSuggestBox looks fishy to me as well. Maybe TypeAheadListBox
should just be a separate widget and not share a class ancestry with
ListModelSuggestBox.
Also, has this been verified? It looks to me like it might have broken
functionality in HostBondPopupView that was using ListModelSuggestBox.
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/ListModelSuggestBox.java
Line 10:
Line 11: /**
Line 12: * SuggestBox widget that adapts to UiCommon list model items. Expects
all of it's items to be non null Strings
Line 13: */
Line 14: public class ListModelSuggestBox<T> extends BaseListModelSuggestBox<T>
{
I still think that this does not make sense for BaseListModelSuggestBox, but it
definitely doesn't make sense for ListModelSuggestBox. The user can input text
freely, and there's generally no way to construct an object of type T from a
String (how would you construct a StoragePool just by its name?).
Line 15:
Line 16: public ListModelSuggestBox() {
Line 17: super(new MultiWordSuggestOracle());
Line 18: initWidget(asSuggestBox());
Line 27: }
Line 28:
Line 29: @Override
Line 30: public void setAcceptableValues(Collection<T> values) {
Line 31: Collection<String> stringValues = Linq.cast(values);
This, for example, wouldn't work for a general T.
Line 32: MultiWordSuggestOracle suggestOracle =
(MultiWordSuggestOracle) asSuggestBox().getSuggestOracle();
Line 33: suggestOracle.clear();
Line 34: suggestOracle.addAll(stringValues);
Line 35: suggestOracle.setDefaultSuggestionsFromText(stringValues);
Line 36: }
Line 37:
Line 38: @Override
Line 39: protected void render(T value, boolean fireEvents) {
Line 40: asSuggestBox().setValue((String) value, fireEvents);
Nor this.
Line 41: }
Line 42:
--
To view, visit http://gerrit.ovirt.org/14936
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I956af3c675894c850a1a104a81cec49f4bd62011
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
