Alona Kaplan has posted comments on this change.

Change subject: webadmin: Focus on label widget by default
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.ovirt.org/#/c/32096/1/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/AddRemoveRowWidget.java
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/AddRemoveRowWidget.java:

Line 310:     @Override
Line 311:     public void focusInput() {
Line 312:         super.focusInput();
Line 313: 
Line 314:         ListIterator<Pair<T, V>> last = 
items.listIterator(items.size());
I think the following code is more readable-

if (!items.isEmpty()){
   V widget = items.get(items.size()-1).getSecond();
   if (widget instanceof Focusable) {
        ((Focusable) widget).setFocus(true);
   }
}
But, as you wish...
Line 315:         if (last.hasPrevious()) {
Line 316:             V widget = last.previous().getSecond();
Line 317:             if (widget instanceof Focusable) {
Line 318:                 ((Focusable) widget).setFocus(true);


http://gerrit.ovirt.org/#/c/32096/1/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/generic/ListModelSuggestBox.java
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/generic/ListModelSuggestBox.java:

Line 23:         asSuggestBox().getValueBox().addFocusHandler(new 
FocusHandler() {
Line 24: 
Line 25:             @Override
Line 26:             public void onFocus(FocusEvent event) {
Line 27:                 Scheduler.get().scheduleDeferred(new 
ScheduledCommand() {
I don't like the use of scheduleDeferred.
You rely on that the suggestBox will be attached and displayed when the 
scheduleDeffered->execute is called. But Maybe it isn't.
Maybe it isn't even focused any more.
I prefer the code will be specific-
onFocus-> is the suggestbox is attached-> showSuggestionList.
onLoad->if the suggestBox is focused->showSuggestionList.
Line 28: 
Line 29:                     @Override
Line 30:                     public void execute() {
Line 31:                         asSuggestBox().showSuggestionList();           
             


Line 27:                 Scheduler.get().scheduleDeferred(new 
ScheduledCommand() {
Line 28: 
Line 29:                     @Override
Line 30:                     public void execute() {
Line 31:                         asSuggestBox().showSuggestionList();           
             
Please format
Line 32:                     }
Line 33:                 });
Line 34:             }
Line 35:         });


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I36c9ae0b7ac9d61112ace1829434c9c685b85a22
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <[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