Alona Kaplan has posted comments on this change.

Change subject: webadmin: Refactored code concerning Neutron agent
......................................................................


Patch Set 3: (7 inline comments)

....................................................
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/provider/NeutronAgentSection.java
Line 20: import com.google.gwt.user.client.ui.FlowPanel;
Line 21: import com.google.gwt.user.client.ui.Label;
Line 22: import com.google.inject.Inject;
Line 23: 
Line 24: public class NeutronAgentSection extends 
AbstractModelBoundPopupWidget<NeutronAgentBehavior> {
I would call it NeutronAgentWidget
Line 25: 
Line 26:     interface Driver extends 
SimpleBeanEditorDriver<NeutronAgentBehavior, NeutronAgentSection> {
Line 27:     }
Line 28: 


Line 85:         driver.initialize(this);
Line 86:     }
Line 87: 
Line 88:     @Override
Line 89:     public void edit(final NeutronAgentBehavior behavior) {
I prefer the terminology model over behavior.
Line 90:         qpidTitle.setVisible(behavior.getIsAvailable());
Line 91:         interfaceMappings.setLabel((String) 
behavior.getInterfaceMappingsLabel().getEntity());
Line 92:         behavior.getPropertyChangedEvent().addListener(new 
IEventListener() {
Line 93: 


Line 93: 
Line 94:             @Override
Line 95:             public void eventRaised(Event ev, Object sender, EventArgs 
args) {
Line 96:                 if ("IsAvailable".equals(((PropertyChangedEventArgs) 
args).PropertyName)) { //$NON-NLS-1$
Line 97:                     qpidTitle.setVisible(behavior.getIsAvailable());
you can use EntityModelLabelEditor to control the changeability from the model.
Line 98:                 }
Line 99:             }
Line 100:         });
Line 101:         
behavior.getInterfaceMappingsLabel().getEntityChangedEvent().addListener(new 
IEventListener() {


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/providers/NeutronAgentBehavior.java
Line 13: import org.ovirt.engine.ui.uicompat.EventArgs;
Line 14: import org.ovirt.engine.ui.uicompat.IEventListener;
Line 15: import org.ovirt.engine.ui.uicompat.PropertyChangedEventArgs;
Line 16: 
Line 17: public class NeutronAgentBehavior extends EntityModel {
I prefer the name NeutronAgentModel
Line 18: 
Line 19:     private static final String QPID_PORT_DEFAULT = "5672"; 
//$NON-NLS-1$
Line 20: 
Line 21:     private final ListModel type;


Line 94:         getPropertyChangedEvent().addListener(new IEventListener() {
Line 95: 
Line 96:             @Override
Line 97:             public void eventRaised(Event ev, Object sender, EventArgs 
args) {
Line 98:                 if ("IsAvailable".equals(((PropertyChangedEventArgs) 
args).PropertyName)) { //$NON-NLS-1$
I think that changing the availability of the model is enough. You shouldn't 
make each one of the model components unavailable.

The view that is using the model should make the whole panel that reflects the 
model visible/invisible according to the model's availability.
Line 99:                     boolean available = getIsAvailable();
Line 100:                     getInterfaceMappings().setIsAvailable(available);
Line 101:                     getQpidHost().setIsAvailable(available);
Line 102:                     getQpidPort().setIsAvailable(available);


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/providers/NeutronPluginTranslator.java
Line 31:     }
Line 32: 
Line 33:     public static String getDisplayStringForPluginName(String 
pluginName) {
Line 34:         try {
Line 35:             return 
EnumTranslator.createAndTranslate(OpenstackNetworkPluginType.valueOf(pluginName));
Why don't you get it from the map?
Line 36:         }
Line 37:         catch (IllegalArgumentException e) {
Line 38:             return pluginName;
Line 39:         }


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/providers/ProviderModel.java
Line 57:     private ListModel pluginType = new ListModel();
Line 58:     private UICommand testCommand;
Line 59:     private EntityModel testResult = new EntityModel();
Line 60: 
Line 61:     private NeutronAgentBehavior neutronAgentTab;
I prefer the name neutronAgentModel.
Line 62: 
Line 63:     public EntityModel getName() {
Line 64:         return name;
Line 65:     }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbec5113f146e4955945d6913c5b11eb60ca2389
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to