Lior Vernia has posted comments on this change.
Change subject: userportal,webadmin: type ahead list box
......................................................................
Patch Set 8: (6 inline comments)
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/BaseListModelSuggestBox.java
Line 23: public abstract class BaseListModelSuggestBox<T> extends Composite
implements EditorWidget<T, TakesConstrainedValueEditor<T>>,
HasConstrainedValue<T> {
Line 24:
Line 25: private TakesConstrainedValueEditor<T> editor;
Line 26:
Line 27: private T value;
This is only needed in TypeAhead, see comment below about making methods
abstract.
Line 28:
Line 29: private SuggestBox suggestBox;
Line 30:
Line 31: private ListModelSuggestionDisplay suggestionDisplay = new
ListModelSuggestionDisplay();
Line 32:
Line 33: private ValueChangeHandler<T> handler;
Line 34:
Line 35: public BaseListModelSuggestBox(MultiWordSuggestOracle
suggestOracle) {
Line 36: suggestBox = new SuggestBox(suggestOracle, new TextBox(),
suggestionDisplay) {
What happens if the z-index isn't set? I didn't see that it was hidden in the
bond interface popup, under what conditions will it be hidden?
Line 37: @Override
Line 38: public void setText(String text) {
Line 39: super.setText(text);
Line 40:
Line 140: this.value = value;
Line 141: render(value, fireEvents);
Line 142: }
Line 143:
Line 144: protected abstract void render(T value, boolean fireEvents);
No problem, it's a matter of personal taste.
Line 145:
Line 146: protected void postTextSet(String text) {
Line 147: // by default do nothing
Line 148: }
Line 152: return value;
Line 153: }
Line 154:
Line 155: @Override
Line 156: public HandlerRegistration addValueChangeHandler(final
ValueChangeHandler<T> handler) {
I don't see the problem in the case of TypeAhead, it could just check whether
the new value is null or not. SuggestBox is indeed a problem that I didn't
think of. How about making getValue(), setValue() and addValueHandler()
abstract, keep the original implementation in SuggestBox and do whatever you
want in TypeAhead? This would also make the no-op render method in SuggestBox
unnecessary, you could define render only in TypeAhead.
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 163: }
Line 164: });
Line 165: }
Line 166:
Line 167: class ListModelSuggestionDisplay extends DefaultSuggestionDisplay
{
Depends on whether the z-index setting is actually needed, I'm not sure I
understand, see my comment above.
Line 168:
Line 169: public ListModelSuggestionDisplay() {
Line 170: // not not be hidden under the panel
Line 171: getPopupPanel().getElement().getStyle().setZIndex(1);
Line 166:
Line 167: class ListModelSuggestionDisplay extends DefaultSuggestionDisplay
{
Line 168:
Line 169: public ListModelSuggestionDisplay() {
Line 170: // not not be hidden under the panel
Typo (double not)?
Line 171: getPopupPanel().getElement().getStyle().setZIndex(1);
Line 172: }
Line 173:
Line 174: // just to make it public
--
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 <[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
