Tomas Jelinek has posted comments on this change.

Change subject: engine, restapi, webadmin: Ability to dismiss alerts and events 
from web-admin portal
......................................................................


Patch Set 4:

(4 comments)

http://gerrit.ovirt.org/#/c/26722/4/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/column/ImageButtonCell.java
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/column/ImageButtonCell.java:

Line 28:     private String elementIdPrefix = DOM.createUniqueId();
Line 29:     private String columnId;
Line 30: 
Line 31:     public ImageButtonCell(ImageResource image) {
Line 32:         super("click"); //$NON-NLS-1$
please use BrowserEvents.CLICK instead of string
Line 33:         this.imageHtml = 
SafeHtmlUtils.fromTrustedString(AbstractImagePrototype.create(image).getHTML());
Line 34:     }
Line 35: 
Line 36:     public void setElementIdPrefix(String elementIdPrefix) {


Line 49:         if (!Element.is(eventTarget)) {
Line 50:             return;
Line 51:         }
Line 52: 
Line 53:         if ("click".equals(event.getType())) { //$NON-NLS-1$
same
Line 54:             onClick(value);
Line 55:         }
Line 56:     }
Line 57: 


Line 69:     /**
Line 70:      *
Line 71:      * @param value
Line 72:      * @return
Line 73:      */
not sure this comment brings too much value ;)

I'd remove it completely since the getTitle() is descriptive enough.
Line 74:     protected abstract String getTitle(T value);
Line 75: 
Line 76:     /**
Line 77:      * Get the UICommand associated with the button.


http://gerrit.ovirt.org/#/c/26722/4/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/configure/roles_ui/RoleTreeView.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/configure/roles_ui/RoleTreeView.java:

Line 293:                                 new RoleNode(ActionGroup.LOGIN, 
getConstants().allowToLoginToTheSystemRoleTreeTooltip()),
Line 294:                                 new 
RoleNode(ActionGroup.TAG_MANAGEMENT, getConstants().allowToManageTags()),
Line 295:                                 new 
RoleNode(ActionGroup.BOOKMARK_MANAGEMENT, 
getConstants().allowToManageBookmarks()),
Line 296:                                 new 
RoleNode(ActionGroup.EVENT_NOTIFICATION_MANAGEMENT, 
getConstants().allowToManageEventNotifications()),
Line 297:                                 new 
RoleNode(ActionGroup.AUDIT_LOG_MANAGEMENT, 
getConstants().allowToManageAuditLogs()),
You need to put the corresponding entry also to 
LocalizedEnums.(java,properties) to have it translatable.
Line 298:                                 new 
RoleNode(ActionGroup.CONFIGURE_ENGINE,
Line 299:                                         
getConstants().allowToGetOrSetSystemConfigurationRoleTreeTooltip()) }));
Line 300:     }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3873226b64049170743d4bf0add531d0fda2a0cb
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@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

Reply via email to