Tomas Jelinek has posted comments on this change.
Change subject: userportal,webadmin: type ahead list box
......................................................................
Patch Set 6: (4 inline comments)
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/BaseListModelSuggestBox.java
Line 19:
Line 20: /**
Line 21: * Base SuggestBox widget that adapts to UiCommon list model items.
Line 22: */
Line 23: public abstract class BaseListModelSuggestBox<T> extends Composite
implements EditorWidget<T, TakesConstrainedValueEditor<T>>,
HasConstrainedValue<T> {
> I don't see a situation where T could be anything that isn't a superclass of
> String,
> because in general T objects will have to be built from user-input Strings.
Not true. Please have a look at the usage of this in this very same patch
(AbstractVmPopupWidget). Only in this very patch T can be:
- StoragePool
- VDSGroup
- Quota
- VmTemplate
> ... but I don't think there's good reason to make BaseListModelSuggestBox
> generic.
Well, it is (look at the comment above). Also, each editor in gwt-common should
(end do) edit models, not plane strings.
Line 24:
Line 25: private TakesConstrainedValueEditor<T> editor;
Line 26:
Line 27: private T value;
Line 29: private SuggestBox suggestBox;
Line 30:
Line 31: private ListModelSuggestionDisplay suggestionDisplay = new
ListModelSuggestionDisplay();
Line 32:
Line 33: private ValueChangeHandler<T> handler;
because it is used later
Line 34:
Line 35: public BaseListModelSuggestBox(MultiWordSuggestOracle
suggestOracle) {
Line 36: suggestBox = new SuggestBox(suggestOracle, new TextBox(),
suggestionDisplay) {
Line 37: @Override
Line 121: public void setEnabled(boolean enabled) {
Line 122: asTextBox().setEnabled(enabled);
Line 123: }
Line 124:
Line 125: public void setValue(T value) {
not true. The underlying model would never be notified that something has
changed
Line 126: if (value != null && value != getValue()) {
Line 127: handler.onValueChange(new ValueChangeEvent<T>(value) {
Line 128: });
Line 129: }
Line 157: this.handler = handler;
Line 158: return asSuggestBox().addValueChangeHandler(new
ValueChangeHandler<String>() {
Line 159: @Override
Line 160: public void onValueChange(ValueChangeEvent<String> event)
{
Line 161: handler.onValueChange(new
ValueChangeEvent<T>(getValue()) {
1: It is set in the correct order. The event.getValue() returns a String while
we use model objects, so we would have to parse it again which does not make
sense.
2: well, you can be pretty sure that the T is not a String => I can not use
event.getValue()
Line 162: });
Line 163: }
Line 164: });
Line 165: }
--
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: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Frank Kobzik <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
