Alona Kaplan has posted comments on this change. Change subject: webadmin: edit vfsConfig- networks table ......................................................................
Patch Set 32: (5 comments) https://gerrit.ovirt.org/#/c/36355/32/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationConstants.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationConstants.java: Line 4302: Line 4303: @DefaultStringValue("Allowed Networks") Line 4304: String allowedNetworks(); Line 4305: Line 4306: @DefaultStringValue("Select Network/s") > "Select Network(s)"? Done Line 4307: String selectNetworks(); Line 4308: Line 4309: @DefaultStringValue("Network") Line 4310: String vfsConfigNetworkName(); https://gerrit.ovirt.org/#/c/36355/32/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/host/VfsConfigWidget.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/host/VfsConfigWidget.java: Line 97: Line 98: ApplicationTemplates templates; Line 99: Line 100: @Inject Line 101: public VfsConfigWidget(ApplicationConstants constants, final ApplicationMessages messages, ApplicationTemplates templates) { > Note that there's a new convention on how to inject these "assets" - see ho Done Line 102: this.constants = constants; Line 103: this.templates = templates; Line 104: Line 105: maxVfsLabel = new ValueLabel<>(new AbstractRenderer<Integer>() { Line 191: Line 192: } Line 193: Line 194: private final class AttachedIndicatorCheckboxColumn extends AbstractCheckboxColumn<VfsConfigNetwork> { Line 195: private AttachedIndicatorCheckboxColumn(AttachedIndicatorFieldUpdater attachedIndicatorFieldUpdater) { > Not a big deal, but this can be a no-arg constructor (it's always the same Done Line 196: super(attachedIndicatorFieldUpdater); Line 197: } Line 198: Line 199: @Override Line 231: public Boolean getValue() { Line 232: boolean allEntriesDisabled = !isEnabled(); Line 233: for (VfsConfigNetwork vfsConfigNetwork : getNetworksTableItems()) { Line 234: if (allEntriesDisabled || canEditAssign(vfsConfigNetwork)) { Line 235: if (!vfsConfigNetwork.isAttached()) { > This can be put as an && in the enclosing condition and remove one level of Done Line 236: return false; Line 237: } Line 238: } Line 239: } https://gerrit.ovirt.org/#/c/36355/32/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/host/VfsConfigWidget.ui.xml File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/host/VfsConfigWidget.ui.xml: Line 65: <g:Label ui:field="allowedNetworksLabel" addStyleNames="{style.allAllowedLabel}" /> Line 66: <e:ListModelRadioGroupEditor ui:field="allNetworksSelectorEditor" addStyleNames="{style.allAllowedSelector}" /> Line 67: <g:SimplePanel addStyleNames="{style.clear}" /> Line 68: <g:FlowPanel ui:field="allowedNetworksPanel" addStyleNames="{style.allowedNetworksPanel}" > Line 69: <g:Label ui:field="selectNetworksLabel" addStyleNames="{style.selectNetworksLabel}" /> > Is this really needed? You already have the allowed networks label above, a Was discussed in the design review level. Decided it is clearer this way. Line 70: <g:ScrollPanel addStyleNames="{style.dock}"> Line 71: <e:EntityModelCellTable ui:field="networks" /> Line 72: </g:ScrollPanel> Line 73: </g:FlowPanel> -- To view, visit https://gerrit.ovirt.org/36355 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2fe3404bd458fee0a4105fa41aa35e945a6d95c9 Gerrit-PatchSet: 32 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@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
