Alona Kaplan has posted comments on this change. Change subject: webadmin: Adding external status to UI ......................................................................
Patch Set 9: (5 comments) https://gerrit.ovirt.org/#/c/40999/9/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/cell/HostAdditionalStatusCell.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/cell/HostAdditionalStatusCell.java: Line 54: return; Line 55: } Line 56: imagesHtml.add(SafeHtmlUtils.fromTrustedString(AbstractImagePrototype.create(statusImage).getHTML())); Line 57: } Line 58: if (! imagesHtml.isEmpty()) { Please remove the redundant space (between ! and imagesHtml). Line 59: SafeHtml safeHtml = MultiImageColumnHelper.getValue(imagesHtml); Line 60: sb.append(templates.hostAdditionalStatusIcon(id, safeHtml)); Line 61: } Line 62: } Line 56: imagesHtml.add(SafeHtmlUtils.fromTrustedString(AbstractImagePrototype.create(statusImage).getHTML())); Line 57: } Line 58: if (! imagesHtml.isEmpty()) { Line 59: SafeHtml safeHtml = MultiImageColumnHelper.getValue(imagesHtml); Line 60: sb.append(templates.hostAdditionalStatusIcon(id, safeHtml)); If you will follow my comment on the columns code, you won't need to use templates.hostAdditionalStatusIcon (SafeHtmlCell already rendered with a div). Line 61: } Line 62: } https://gerrit.ovirt.org/#/c/40999/9/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/column/HostAdditionalStatusColumn.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/column/HostAdditionalStatusColumn.java: Line 15: Line 16: import java.util.LinkedHashMap; Line 17: import java.util.Map; Line 18: Line 19: public class HostAdditionalStatusColumn<S> extends AbstractColumn<S, VDS> { I would extend AbstractSafeHtmlColumn (You can refer to MainTabNetworkView-roleColumn for an example). Doing that, you won't need to deal with the cell. Just to override to getValue(..) and getTooltip(..) methods on the column. Line 20: Line 21: private final static ApplicationConstants constants = AssetProvider.getConstants(); Line 22: private final static ApplicationResources resources = AssetProvider.getResources(); Line 23: Line 41: return null; Line 42: } Line 43: Map<SafeHtml, String> imagesToText = new LinkedHashMap<>(); Line 44: Line 45: if (host.isUpdateAvailable()) { Duplicate code (the cell's render method contains the same logic). Please extract it to a method (if you will follow my previous comment the cells render method code would be moved to this class, so you will be easily able to use the same method). Line 46: imagesToText.put(SafeHtmlUtils.fromTrustedString(AbstractImagePrototype.create(resources.updateAvailableImage()) Line 47: .getHTML()), constants.updateAvailable()); Line 48: } Line 49: if (host.getExternalStatus() != ExternalStatus.Ok) { Line 66: } Line 67: imagesToText.put(SafeHtmlUtils.fromTrustedString(AbstractImagePrototype.create(statusImage).getHTML()), Line 68: constants.ExternalStatus() + host.getExternalStatus().name()); Line 69: } Line 70: if (! imagesToText.isEmpty()) { space (! images) Line 71: return MultiImageColumnHelper.getTooltip(imagesToText); Line 72: } Line 73: return null; Line 74: } -- To view, visit https://gerrit.ovirt.org/40999 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I99e7066176e70ec94f543aa1120bb355acad9b0a Gerrit-PatchSet: 9 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eli Mesika <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
