Vojtech Szocs has posted comments on this change. Change subject: engine: Add branded link to console resources ......................................................................
Patch Set 7: (3 comments) .................................................... File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/view/popup/ConsolePopupView.java Line 192: rdpPluginImplRadioButton = new EntityModelRadioButtonEditor("4"); //$NON-NLS-1$ Line 193: rdpPluginImplRadioButton.setLabel(constants.browserPlugin()); Line 194: Line 195: clientConsoleResourcesUrl = new Anchor(dynamicMessages.consoleClientResources()); Line 196: clientConsoleResourcesUrl.addClickHandler(new ClickHandler() { +1 to comment from Alex Frantisek, handler registrations should be generally done within (ConsolePopup)PresenterWidget, i.e. ViewDef has method that returns HasClickHandlers (or similar handler registration interface), View implements this method by returning the specific widget (like Anchor) and PresenterWidget does "registerHandler(getView().getClientConsoleResourcesLink().addClickHandler(...))" within its onBind method. View should be as logic-less (dumb) as possible :-) Line 197: @Override Line 198: public void onClick(ClickEvent event) { Line 199: Window.open(dynamicMessages.consoleClientResourcesUrl(), "_blank", null); //$NON-NLS-1$ Line 200: } .................................................... File frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/section/main/view/tab/basic/MainTabBasicDetailsView.java Line 121: return AsyncDataProvider.getOsName(object); Line 122: } Line 123: }); Line 124: consoleClientResourcesUrl = new Anchor(dynamicMessages.clientResources()); Line 125: consoleClientResourcesUrl.addClickHandler(new ClickHandler() { I agree with Alex, please see my comment in ConsolePopupView.java - Presenter(Widget) should perform handler registration via ViewDef interface. Line 126: @Override Line 127: public void onClick(ClickEvent event) { Line 128: Window.open(dynamicMessages.consoleClientResourcesUrl(), "_blank", null); //$NON-NLS-1$ Line 129: } .................................................... File packaging/branding/ovirt.brand/messages.properties Line 14: obrand.common.version_about=oVirt Engine Version: {0} Line 15: # Console Client Resources URL Line 16: obrand.common.console_client_resources=Console Client Resources Line 17: obrand.common.client_resources=Client Resources Line 18: obrand.common.console_client_resources_url=http://www.ovirt.org/Console_Client_Resources Hm but isn't "obrand.common.console_client_resources_url" meant to be customized too? Line 19: Line 20: # User portal specific messages that can be over written. Line 21: Line 22: # The application title. -- To view, visit http://gerrit.ovirt.org/18596 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5f7316e6ce5032123edc2e39b4939d0c8dab1cb1 Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> 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