Vojtech Szocs has posted comments on this change.

Change subject: webadmin: cluster guide me datacenter
......................................................................


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/37763/1/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/EntityModelCellTable.java
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/EntityModelCellTable.java:

Line 221:                     return;
Line 222:                 }
Line 223: 
Line 224:                 getListModel().setSelectedItems(null);
Line 225:                 getListModel().setSelectedItem(null);
> Whenever the checkbox column is visible it's created as a radio button, see
I agree with Daniel and Lior, this fix seems as the right thing to do.
Line 226:                 // Clear "IsSelected"
Line 227:                 for (EntityModel entity : (List<EntityModel>) 
getListModel().getItems()) {
Line 228:                     entity.setIsSelected(false);
Line 229:                 }


https://gerrit.ovirt.org/#/c/37763/1/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterGuideModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterGuideModel.java:

Line 218:     private void updateOptions() {
Line 219:         getCompulsoryActions().clear();
Line 220:         getOptionalActions().clear();
Line 221: 
Line 222:         startProgress(null);
What if getEntity() returns null at this point? It would mean that none of the 
if-else clauses would execute, meaning the Cluster Guide dialog would end up 
being stuck with progress animation.

Maybe we could do:

 if (getEntity() == null) {
     return;
 }
 startProgress(null);

and simplify the if-else conditions below?
Line 223:         if (getEntity() != null && getEntity().getStoragePoolId() != 
null) {
Line 224:             //Datacenter associated with this cluster.
Line 225:             AsyncDataProvider.getInstance().getDataCenterById(new 
AsyncQuery(this, new INewAsyncCallback() {
Line 226:                     @Override


-- 
To view, visit https://gerrit.ovirt.org/37763
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifaafdd5593d3721b9e6a8b466b2ac186b18bd6fe
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Greg Sheremeta <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[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