Vojtech Szocs has posted comments on this change. Change subject: userportal, webadmin: add element ids to all general subtabs ......................................................................
Patch Set 3: (15 comments) Some more comments, I noticed that AbstractSubTabFormView constructor already calls generateIds() so calling it again in subclass might not be necessary. Let me know what you think. http://gerrit.ovirt.org/#/c/34800/3/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/AbstractFormPanel.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/AbstractFormPanel.java: Line 102: Grid getDetailView(int column) { Line 103: return detailViews.get(column); Line 104: } Line 105: Line 106: public void setElementId(String elementId) { I think we should add @Override here, since setElementId method above implements HasElementId.setElementId interface method. @Override public void setElementId(String elementId) { ... } Line 107: this.elementId = elementId; Line 108: } Line 109: http://gerrit.ovirt.org/#/c/34800/3/frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/section/main/view/tab/extended/template/SubTabExtendedTemplateGeneralView.java File frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/section/main/view/tab/extended/template/SubTabExtendedTemplateGeneralView.java: Line 37: super(modelProvider); Line 38: form = new TemplateGeneralModelForm(modelProvider, constants); Line 39: initWidget(ViewUiBinder.uiBinder.createAndBindUi(this)); Line 40: Line 41: generateIds(); Hm, invoking super() above should already yield call to generateIds(), see AbstractSubTabFormView constructor. Do we need to call generateIds() again here? Line 42: Line 43: form.initialize(); Line 44: } Line 45: http://gerrit.ovirt.org/#/c/34800/3/frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/section/main/view/tab/extended/vm/SubTabExtendedPoolGeneralView.java File frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/section/main/view/tab/extended/vm/SubTabExtendedPoolGeneralView.java: Line 37: super(modelProvider); Line 38: form = new PoolGeneralModelForm(modelProvider, constants); Line 39: initWidget(ViewUiBinder.uiBinder.createAndBindUi(this)); Line 40: Line 41: generateIds(); Hm, invoking super() above should already yield call to generateIds(), see AbstractSubTabFormView constructor. Do we need to call generateIds() again here? Line 42: Line 43: form.initialize(); Line 44: } Line 45: http://gerrit.ovirt.org/#/c/34800/3/frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/section/main/view/tab/extended/vm/SubTabExtendedVmGeneralView.java File frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/section/main/view/tab/extended/vm/SubTabExtendedVmGeneralView.java: Line 37: super(modelProvider); Line 38: form = new VmGeneralModelForm(modelProvider, constants); Line 39: initWidget(ViewUiBinder.uiBinder.createAndBindUi(this)); Line 40: Line 41: generateIds(); Hm, invoking super() above should already yield call to generateIds(), see AbstractSubTabFormView constructor. Do we need to call generateIds() again here? Line 42: Line 43: form.initialize(); Line 44: } Line 45: http://gerrit.ovirt.org/#/c/34800/3/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/cluster/SubTabClusterGeneralView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/cluster/SubTabClusterGeneralView.java: Line 95: Line 96: this.form = new ClusterGeneralModelForm(modelProvider, constants); Line 97: Line 98: // generate ids Line 99: generateIds(); Hm, invoking super() above should already yield call to generateIds(), see AbstractSubTabFormView constructor. Do we need to call generateIds() again here? Line 100: Line 101: // init form Line 102: form.initialize(); Line 103: http://gerrit.ovirt.org/#/c/34800/3/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/disk/SubTabDiskGeneralView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/disk/SubTabDiskGeneralView.java: Line 64: Line 65: initWidget(ViewUiBinder.uiBinder.createAndBindUi(this)); Line 66: driver.initialize(this); Line 67: Line 68: generateIds(); Hm, invoking super() above should already yield call to generateIds(), see AbstractSubTabFormView constructor. Do we need to call generateIds() again here? Line 69: Line 70: // Build a form using the FormBuilder Line 71: formBuilder = new FormBuilder(formPanel, 1, 7); Line 72: http://gerrit.ovirt.org/#/c/34800/3/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/gluster/SubTabVolumeGeneralView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/gluster/SubTabVolumeGeneralView.java: Line 64: Line 65: initWidget(formPanel); Line 66: driver.initialize(this); Line 67: Line 68: generateIds(); Hm, invoking super() above should already yield call to generateIds(), see AbstractSubTabFormView constructor. Do we need to call generateIds() again here? Line 69: Line 70: // Build a form using the FormBuilder Line 71: formBuilder = new FormBuilder(formPanel, 1, 7); Line 72: http://gerrit.ovirt.org/#/c/34800/3/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/host/SubTabHostGeneralView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/host/SubTabHostGeneralView.java: Line 140: Line 141: initWidget(ViewUiBinder.uiBinder.createAndBindUi(this)); Line 142: driver.initialize(this); Line 143: Line 144: generateIds(); Hm, invoking super() above should already yield call to generateIds(), see AbstractSubTabFormView constructor. Do we need to call generateIds() again here? Line 145: Line 146: boolean virtSupported = ApplicationModeHelper.isModeSupported(ApplicationMode.VirtOnly); Line 147: boolean glusterSupported = ApplicationModeHelper.isModeSupported(ApplicationMode.GlusterOnly); Line 148: http://gerrit.ovirt.org/#/c/34800/3/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/network/SubTabNetworkGeneralView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/network/SubTabNetworkGeneralView.java: Line 68: Line 69: initWidget(ViewUiBinder.uiBinder.createAndBindUi(this)); Line 70: driver.initialize(this); Line 71: Line 72: generateIds(); Hm, invoking super() above should already yield call to generateIds(), see AbstractSubTabFormView constructor. Do we need to call generateIds() again here? Line 73: Line 74: // Build a form using the FormBuilder Line 75: formBuilder = new FormBuilder(formPanel, 2, 4); Line 76: http://gerrit.ovirt.org/#/c/34800/3/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/pool/SubTabPoolGeneralView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/pool/SubTabPoolGeneralView.java: Line 76: Line 77: initWidget(ViewUiBinder.uiBinder.createAndBindUi(this)); Line 78: driver.initialize(this); Line 79: Line 80: generateIds(); Hm, invoking super() above should already yield call to generateIds(), see AbstractSubTabFormView constructor. Do we need to call generateIds() again here? Line 81: Line 82: // Build a form using the FormBuilder Line 83: formBuilder = new FormBuilder(formPanel, 3, 6); Line 84: http://gerrit.ovirt.org/#/c/34800/3/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/provider/SubTabProviderGeneralView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/provider/SubTabProviderGeneralView.java: Line 62: Line 63: initWidget(ViewUiBinder.uiBinder.createAndBindUi(this)); Line 64: driver.initialize(this); Line 65: Line 66: generateIds(); Hm, invoking super() above should already yield call to generateIds(), see AbstractSubTabFormView constructor. Do we need to call generateIds() again here? Line 67: Line 68: // Build a form using the FormBuilder Line 69: formBuilder = new FormBuilder(formPanel, 1, 4); Line 70: http://gerrit.ovirt.org/#/c/34800/3/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/storage/SubTabStorageGeneralView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/storage/SubTabStorageGeneralView.java: Line 89: Line 90: initWidget(ViewUiBinder.uiBinder.createAndBindUi(this)); Line 91: driver.initialize(this); Line 92: Line 93: generateIds(); Hm, invoking super() above should already yield call to generateIds(), see AbstractSubTabFormView constructor. Do we need to call generateIds() again here? Line 94: Line 95: // Build a form using the FormBuilder Line 96: formBuilder = new FormBuilder(formPanel, 1, 12); Line 97: http://gerrit.ovirt.org/#/c/34800/3/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/template/SubTabTemplateGeneralView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/template/SubTabTemplateGeneralView.java: Line 37: super(modelProvider); Line 38: this.form = new TemplateGeneralModelForm(modelProvider, constants); Line 39: initWidget(ViewUiBinder.uiBinder.createAndBindUi(this)); Line 40: Line 41: generateIds(); Hm, invoking super() above should already yield call to generateIds(), see AbstractSubTabFormView constructor. Do we need to call generateIds() again here? Line 42: Line 43: form.initialize(); Line 44: } Line 45: http://gerrit.ovirt.org/#/c/34800/3/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/user/SubTabUserGeneralView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/user/SubTabUserGeneralView.java: Line 60: Line 61: initWidget(ViewUiBinder.uiBinder.createAndBindUi(this)); Line 62: driver.initialize(this); Line 63: Line 64: generateIds(); Hm, invoking super() above should already yield call to generateIds(), see AbstractSubTabFormView constructor. Do we need to call generateIds() again here? Line 65: Line 66: // Build a form using the FormBuilder Line 67: formBuilder = new FormBuilder(formPanel, 1, 3); Line 68: http://gerrit.ovirt.org/#/c/34800/3/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/virtualMachine/SubTabVirtualMachineGeneralView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/virtualMachine/SubTabVirtualMachineGeneralView.java: Line 63: Line 64: initWidget(ViewUiBinder.uiBinder.createAndBindUi(this)); Line 65: clearAlerts(); Line 66: Line 67: generateIds(); Hm, invoking super() above should already yield call to generateIds(), see AbstractSubTabFormView constructor. Do we need to call generateIds() again here? Line 68: Line 69: form.initialize(); Line 70: } Line 71: -- To view, visit http://gerrit.ovirt.org/34800 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8b281e7b130dd0bbcb23887719e79ab615a901d6 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
