Lior Vernia has posted comments on this change.

Change subject: webadmin: Edit interface- the last label should get the focus
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/28883/1/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/host/NicLabelWidget.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/host/NicLabelWidget.java:

Line 56: 
Line 57:     @Override
Line 58:     protected void onAdd(ListModel<String> value, NicLabelEditor 
widget) {
Line 59:         super.onAdd(value, widget);
Line 60:         widget.setFocus(true);
This is a good fix. However, I think it's only a matter of time until similar 
bugs are opened for other widgets inheriting from AddRemoveRowWidget, so I 
would ask you to make the fix more general. I see two choices:

1. Create a new subclass for AddRemoveRowWidget that requires the Widget of 
type V to also implement Focusable, and does this as part of its onAdd() 
method. Then have NicLabelWidget extend that instead of AddRemoveRowWidget.

2. Add this straight to AddRemoveRowWidget.onAdd(), if the widget is instanceof 
Focusable.

I think the first one is more correct, however I've been contemplating doing 
something similar with HasEnabled, and since there's no multiple inheritance in 
Java, one would have to choose whether they want the HasEnabled behavior or the 
Focusable behavior (where it would often make sense to want both).

What do you think?
Line 61:     }


-- 
To view, visit http://gerrit.ovirt.org/28883
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3ad1d11aab2a833ac210a737bbb95319197c4f1e
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to