Vojtech Szocs has posted comments on this change.

Change subject: userportal: allow links in MOTD
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.ovirt.org/#/c/24006/3/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/presenter/AbstractLoginPopupPresenterWidget.java
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/presenter/AbstractLoginPopupPresenterWidget.java:

Line 41:         void clearErrorMessage();
Line 42: 
Line 43:         HasUiCommandClickHandlers getLoginButton();
Line 44: 
Line 45:         String getMotdAnchor(String url);
Small nitpick: consider method name like getMotdAnchorHtml.
Line 46:     }
Line 47: 
Line 48:     private static final Logger logger = 
Logger.getLogger(AbstractLoginPopupPresenterWidget.class.getName());
Line 49: 


http://gerrit.ovirt.org/#/c/24006/3/frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/section/login/view/LoginPopupView.java
File 
frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/section/login/view/LoginPopupView.java:

Line 269:     }
Line 270: 
Line 271:     @Override
Line 272:     public String getMotdAnchor(String url) {
Line 273:         return new Anchor(url, url, "_blank").toString(); 
//$NON-NLS-1$
This method looks the same in both LoginPopupView classes, consider moving it 
to AbstractLoginPopupView.

In this particular case it's not a big deal, however please note that DOM 
operations, such as creating GWT Anchor widget that triggers creation of <a> 
element, carry performance penalty. If our goal is to just return HTML of MOTD 
<a> element, we can use SafeHtmlTemplates:

 interface MotdAnchorTemplate extends SafeHtmlTemplates {
     @Template("<a href=\"{0}\" target=\"blank\">{1}</a>")
     SafeHtml anchor(String url, String text);
 }

 private static MotdAnchorTemplate template;

 public AbstractLoginPopupView(...) {
     ...
     // Delay template creation until the first time it's needed
     if (template == null) {
         template = GWT.create(MotdAnchorTemplate.class);
     }
 }

 public String getMotdAnchor(String url) {
     return template.anchor(url, url);
 }
Line 274:     }
Line 275: 
Line 276:     public interface Style extends CssResource {
Line 277: 


-- 
To view, visit http://gerrit.ovirt.org/24006
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6c78c9e6d6dc9417e22c71dab66ae4507ea5c191
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@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