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

Reply via email to