Alexander Wels has posted comments on this change.

Change subject: userportal, webadmin: cleanup cells -- use SafeHtmlTemplates
......................................................................


Patch Set 1:

(5 comments)

https://gerrit.ovirt.org/#/c/39110/1/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/LunSelectionCell.java
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/cell/LunSelectionCell.java:

Line 25: 
Line 26:         @Template("<input id=\"{0}\" tabindex='-1' type=\"{1}\" 
checked disabled />")
Line 27:         SafeHtml inputCheckedDisabled(String id, String type);
Line 28: 
Line 29:         @Template("<input id=\"{0}\" tabindex='-1' type=\"{1}\" />")
Can't you do something like this:

  @Template("<input id='{0}' tabindex='-1' type='{1}' {2} {3} />")

And have 2 more parameters, strings for 'checked' and 'disabled'
Line 30:         SafeHtml inputUnchecked(String id, String type);
Line 31: 
Line 32:         @Template("<input id=\"{0}\" tabindex='-1' type=\"{1}\" 
disabled />")
Line 33:         SafeHtml inputUncheckedDisabled(String id, String type);


Line 57:         } else if (!value.getIsAccessible()) {
Line 58:             // ImageResourceCell sets the id
Line 59:             imageCell.render(context, resources.logWarningImage(), sb, 
id);
Line 60:         } else {
Line 61:             boolean checked = value.getIsSelected();
With the above in mind:

String checked = value.getIsSelected ? "checked" : "";
Line 62:             boolean disabled = value.getIsGrayedOut();
Line 63:             String inputId = id + "_input"; //$NON-NLS-1$
Line 64: 
Line 65:             String type = multiSelection ? "checkbox" : "radio"; 
//$NON-NLS-1$ //$NON-NLS-2$


Line 62: boolean
With the above in mind:

String disabled = value.getIsGrayedOut() ? "disabled" : "";


Line 75:                 input = templates.inputUnchecked(inputId, type);
Line 76:             }
Line 77:             else {
Line 78:                 input = templates.inputUncheckedDisabled(inputId, 
type);
Line 79:             }
With the above in mind you can replace all of this with:

input = templates.inputUnchecked(inputId, type, checked, disabled);
Line 80: 
Line 81:             sb.append(templates.span(id, input));
Line 82:         }
Line 83:     }


https://gerrit.ovirt.org/#/c/39110/1/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/cell/CustomSelectionCell.java
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/table/cell/CustomSelectionCell.java:

Line 101:             clearViewData(key);
Line 102:             viewData = null;
Line 103:         }
Line 104: 
Line 105:         if (isEnabled) {
Similar to before have one template with an extra parameter that is blank or 
not depending on the value of the boolean.
Line 106:             sb.append(template.selectEnabled(id, style));
Line 107:         }
Line 108:         else {
Line 109:             sb.append(template.selectDisabled(id, style));


-- 
To view, visit https://gerrit.ovirt.org/39110
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iab169b1daf5974655f90bab47e8ee5286c4f2e51
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Greg Sheremeta <gsher...@redhat.com>
Gerrit-Reviewer: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Greg Sheremeta <gsher...@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