Tomas Jelinek has posted comments on this change.
Change subject: userportal,webadmin: type ahead list box
......................................................................
Patch Set 9: (7 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> {
Why? It is a base class which can be used only as a base class - not directly
instantiate it. I like to name it explicitly. And also it is pretty common in
our codebase to have the base classes prefixed by the 'Base'.
Line 24:
Line 25: private TakesConstrainedValueEditor<T> editor;
Line 26:
Line 27: private T value;
Line 112: public void setEnabled(boolean enabled) {
Line 113: asTextBox().setEnabled(enabled);
Line 114: }
Line 115:
Line 116: public void setValue(T value) {
true. Will do that.
Line 117: this.value = value;
Line 118: render(value, false);
Line 119:
Line 120: }
Line 123: this.value = value;
Line 124: render(value, fireEvents);
Line 125: }
Line 126:
Line 127: protected abstract void render(T value, boolean fireEvents);
Because the setValue is a bridge between this class and SuggestBox.setValue so
needs to have the same params. And the render calls the setValue on SuggestBox
so needs to know if that one should fire events or not.
Line 128:
Line 129: protected abstract T wrap(String value);
Line 130:
Line 131: @Override
Line 125: }
Line 126:
Line 127: protected abstract void render(T value, boolean fireEvents);
Line 128:
Line 129: protected abstract T wrap(String value);
I also don't particularly like the wrap(). Maybe something like asEntity?
Line 130:
Line 131: @Override
Line 132: public T getValue() {
Line 133: return value;
Line 138: return asSuggestBox().addValueChangeHandler(new
ValueChangeHandler<String>() {
Line 139: @Override
Line 140: public void onValueChange(ValueChangeEvent<String> event)
{
Line 141: T value = wrap(event.getValue());
Line 142: if (value != null) {
Because the null means that the new value is not correct so we don't want to
notify the model about it - practically what happens that the text in the box
returns to the original one and no one gets notifyed. Of course this will never
happen for the suggest box side, but it is just a support for this - not an
enforced behavior. So the suggest box side will never return nulll and will
work as expected.
I will document this in the wrap (or how will we call it).
Line 143: handler.onValueChange(new
ValueChangeEvent<T>(wrap(event.getValue())) {
Line 144: });
Line 145: }
Line 146: }
Line 139: @Override
Line 140: public void onValueChange(ValueChangeEvent<String> event)
{
Line 141: T value = wrap(event.getValue());
Line 142: if (value != null) {
Line 143: handler.onValueChange(new
ValueChangeEvent<T>(wrap(event.getValue())) {
yes, I've forgotten it there
Line 144: });
Line 145: }
Line 146: }
Line 147: });
....................................................
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 extends
BaseListModelSuggestBox<Object> {
Actually we prefix each editor's name which edits ListModel by 'ListModel'.
Maybe could be ListModelStringOnlySuggestBox but I don't particularly like the
idea of putting something which should be a generic parameter to the name of
the class.
Line 15:
Line 16: public ListModelSuggestBox() {
Line 17: super(new MultiWordSuggestOracle());
Line 18: initWidget(asSuggestBox());
--
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: 9
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: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
