Daniel Erez has posted comments on this change.

Change subject: userportal,webadmin: type ahead list box
......................................................................


Patch Set 8: (4 inline comments)

Since the new combobox is assembled manually with a captured triangle image,
it looks differently from a browser rendered listbox (i.e. according to the 
browser).
Clarifying screenshot: http://i.imgur.com/YfBgHz4.png

For consistency, we should probably replace the regular listbox triangle as 
well.
Or, simply move 'Based on Template' widget to the top for better 
symmetry/flow...?
What do you think?

....................................................
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> {
not sure, but won't it be simpler to extend SuggestBox instead of Composite?
Line 24: 
Line 25:     private TakesConstrainedValueEditor<T> editor;
Line 26: 
Line 27:     private T value;


....................................................
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/ListModelTypeAheadListBox.java
Line 110:         suggestBox.getTextBox().addFocusHandler(new FocusHandler() {
Line 111: 
Line 112:             @Override
Line 113:             public void onFocus(FocusEvent event) {
Line 114:                 eventHandler =
is it different than event.preventDefault()?
Line 115:                         Event.addNativePreviewHandler(new 
EnterIgnoringNativePreviewHandler<T>(ListModelTypeAheadListBox.this));
Line 116:             }
Line 117:         });
Line 118: 


....................................................
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/AbstractVmPopupWidget.java
Line 103: 
Line 104:         String generalTabExtendedRightWidgetWidth();
Line 105:     }
Line 106: 
Line 107:     interface TypeAheadNameDescriptionTemplate extends 
SafeHtmlTemplates {
extract to CommonApplicationTemplates
Line 108:         @Template("<div style='width: 600px'><div style='width: 30%; 
display: inline-block; border-right: 1px solid black; font-weight:bold; 
padding-right: 25px; float: left'>{0}</div><div style='display: inline-block; 
margin-left: 25px;'>{1}</div></div>")
Line 109:         SafeHtml input(String name, String description);
Line 110:     }
Line 111: 


Line 650: 
Line 651:     @SuppressWarnings({ "rawtypes", "unchecked" })
Line 652:     private void initListBoxEditors() {
Line 653:         // General tab
Line 654:         dataCenterEditor = new 
ListModelTypeAheadListBoxEditor<Object>(
The code looks pretty repetitive. If not too difficult/complicated,
consider extracting it (along with 'typeAheadNameDescriptionTemplateNullSafe' 
method).
Line 655:                 new 
ListModelTypeAheadListBoxEditor.NullSafeSuggestBoxRenderer<Object>() {
Line 656: 
Line 657:                     @Override
Line 658:                     public String getReplacementStringNullSafe(Object 
data) {


--
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]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to