Vojtech Szocs has posted comments on this change.

Change subject: webadmin: changing tab order in HostPopupView
......................................................................


Patch Set 4:

Hi Ravi, I think we should improve this patch a bit before merging.

AbstractModelBoundPopupView now has edit method so all subclasses should call 
"super.edit" for consistent behavior, but this means modifying all subclasses.

The edit method's purpose is to populate GUI (widget values) from given object 
(model), so tab index handling should be done in a different way (i.e. not 
driven by edit method).

I suggest that we solve tab index handling like this:
* add method in AbstractModelBoundPopupView for subclasses to register widgets 
for which tab index should be handled automatically, i.e. protected void 
addWidgetsForTabIndexHandling(Focusable... widgets)
* why UiCommandButton implements FocusableComponentsContainer? instead, 
AbstractUiCommandButton should implement Focusable and delegate implementation 
to getButtonWidget method
* AbstractValidatedWidgetWithLabel (i.e. editor widgets such as 
ListModelListBoxEditor or EntityModelTextBoxEditor) already implements 
Focusable, we are OK here
* in HostPopupView constructor, right after uiBinder creates UI, call 
addWidgetsForTabIndexHandling(dataCenterEditor, ...) -> replaces code in 
HostPopupView.setTabIndexes

So AbstractModelBoundPopupView will handle tab indexes automatically, 
subclasses won't have to call setTabIndex on their own, but just specify 
widgets to handle. Tab indexes will be assigned first within 
AbstractModelBoundPopupPresenterWidget.init and then re-assigned each time new 
footer button is added.

What do you think? Long story short, is it OK if I take over and make above 
mentioned changes?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8b5c7d6a9e7b98c1f5e1fecbaaf193cd862ccb47
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Ravi Nori <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to