Arik Hadas has posted comments on this change. Change subject: webadmin: fix import/general sub-tab ......................................................................
Patch Set 3: (2 comments) comment changed, additional comments inside https://gerrit.ovirt.org/#/c/39586/3/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmImportGeneralModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmImportGeneralModel.java: Line 19: import org.ovirt.engine.ui.uicompat.ConstantsManager; Line 20: import org.ovirt.engine.ui.uicompat.EnumTranslator; Line 21: import org.ovirt.engine.ui.uicompat.PropertyChangedEventArgs; Line 22: Line 23: public class VmImportGeneralModel extends AbstractGeneralModel<ImportVmData> { > 1: in that case you just need to provide the proper implementation of the I Looking in the long-run, these sub-tab differs in the fields they should present - the general sub tab should show everything, including field that will be added in the future, and the import sub-tab should present only fields that are relevant for the import operation, which is a subset of the overall VM fields which is not expected to be changed (unless new general devices such as usb will be introduced in the future). Moreover, in the import sub-tab more fields should probably be editable while in the general sub-tab nothing is editable. That makes these sub-tabs so different that I think they should be different classes Line 24: private static final VmTemplateNameRenderer vmTemplateNameRenderer = new VmTemplateNameRenderer(); Line 25: private static EnumTranslator translator = EnumTranslator.getInstance(); Line 26: Line 27: private String name; https://gerrit.ovirt.org/#/c/39586/3/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/storage/backup/ImportVmGeneralSubTabView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/storage/backup/ImportVmGeneralSubTabView.java: Line 23: import com.google.gwt.user.client.ui.Image; Line 24: import com.google.gwt.user.client.ui.Widget; Line 25: Line 26: public class ImportVmGeneralSubTabView extends AbstractSubTabFormView<VM, ImportVmFromExportDomainModel, VmImportGeneralModel> implements SubTabVirtualMachineGeneralPresenter.ViewDef, Editor<VmImportGeneralModel> { Line 27: > if I see it correctly the only difference is that the SubTabVirtualMachineG I was just about to try to unify them while I realized that the import sub-tab doesn't need the alerts part so I removed it. now they really don't share much, is it ok to keep it that way then? Line 28: interface ViewUiBinder extends UiBinder<Widget, ImportVmGeneralSubTabView> { Line 29: ViewUiBinder uiBinder = GWT.create(ViewUiBinder.class); Line 30: } Line 31: -- To view, visit https://gerrit.ovirt.org/39586 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ied6e9619ba6f539e42600d3650adff2b9272ac9e Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Martin Betak <mbe...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@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
