10:36 p.m.
Author: nbelaevski
Date: 2007-06-03 16:36:52 -0400 (Sun, 03 Jun 2007)
New Revision: 988
Modified:
trunk/sandbox-samples/panelmenu-sample/
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/PanelMenuGroupRenderer.java
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/PanelMenuItemRenderer.java
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/PanelMenuRenderer.java
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/PanelMenuRendererBase.java
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/iconImages/PanelMenuIconArrow.java
trunk/sandbox/panelmenu/src/main/resources/org/richfaces/renderkit/html/scripts/panelMenu.js
trunk/sandbox/panelmenu/src/main/templates/org/richfaces/htmlPanelMenu.jspx
trunk/sandbox/panelmenu/src/main/templates/org/richfaces/htmlPanelMenuGroup.jspx
Log:
Code review notes commited
Modified:
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/PanelMenuGroupRenderer.java
===================================================================
---
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/PanelMenuGroupRenderer.java 2007-06-02
01:45:10 UTC (rev 987)
+++
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/PanelMenuGroupRenderer.java 2007-06-03
20:36:52 UTC (rev 988)
@@ -38,6 +38,7 @@
public class PanelMenuGroupRenderer extends PanelMenuRendererBase {
protected Class getComponentClass() {
+ //TODO by nick - dima - add concrete component class
return UIComponent.class;
}
@@ -72,6 +73,9 @@
String align = "";
UIPanelMenu panelMenu = findMenu(component);
+ //TODO by nick - dima - add isTopLevel() method to group component
+ //TODO by nick - dima - btw, components can be enclosed in other components such as
ui:repeat, so panelManu will be
+ // parent of parent of ...
boolean isTopLevel = component.getParent() instanceof UIPanelMenu;
if (isTopLevel){
@@ -96,12 +100,16 @@
return;
}
+ //TODO by nick - dima - add isTopLevel() method to group component
boolean isTopLevel = component.getParent() instanceof UIPanelMenu;
+ //TODO by nick - dima - component value should be used here
boolean isOpened = isOpened(context,component).equals("opened") ? true :
false;
String source = "";
String iconNode = "";
+ //TODO by nick - dima - access through attributes map is usually slower as done through
reflection. Should be replaced with abstract
+ // getter invocation
Object iconCollapsed = component.getAttributes().get("iconCollapsed");
Object iconExpanded = component.getAttributes().get("iconExpanded");
String icon = (isOpened ? iconExpanded : iconCollapsed).toString();
@@ -117,6 +125,7 @@
int h = 16; //width(context);
StringBuffer imageBuffer = new StringBuffer();
+ //TODO by nick - dima - use
writer.startElement()/writer.endElement()/getUtils().writeAttribute() calls to output
image
imageBuffer.append("<img src=\"" + source + "\" ");
imageBuffer.append("alt=\"\" vspace=\"0\"
hspace=\"0\" ");
imageBuffer.append("style=\"display:block;\" ");
@@ -164,6 +173,7 @@
format.add("display", "none");
if(component.getParent() instanceof UIPanelMenuGroup) {
UIPanelMenuGroup parent = (UIPanelMenuGroup)component.getParent();
+ //TODO by nick - dima - take opened value from component itself
PanelMenuGroupRenderer renderer = (PanelMenuGroupRenderer)
context.getRenderKit().getRenderer(parent.getFamily(), parent.getRendererType());
try {
if ( renderer.isOpened(context, parent).equals("opened") ){
Modified:
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/PanelMenuItemRenderer.java
===================================================================
---
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/PanelMenuItemRenderer.java 2007-06-02
01:45:10 UTC (rev 987)
+++
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/PanelMenuItemRenderer.java 2007-06-03
20:36:52 UTC (rev 988)
@@ -40,6 +40,7 @@
public class PanelMenuItemRenderer extends PanelMenuRendererBase {
protected Class getComponentClass() {
+ //TODO by nick - dima - add concrete component class
return UIComponent.class;
}
@@ -72,6 +73,7 @@
return;
}
String align = "";
+ //TODO by nick - dima - add isTopLevel() method to item component
boolean isTopLevel = component.getParent() instanceof UIPanelMenu;
if (isTopLevel){
@@ -119,6 +121,7 @@
if (!iconType.equals("none")){
int h = 16; //width(context);
+ //TODO by nick - dima - use
writer.startElement()/writer.endElement()/getUtils().writeAttribute() calls to output
image
StringBuffer imageBuffer = new StringBuffer();
imageBuffer.append("<img src=\"" + source + "\" ");
imageBuffer.append("alt=\"\" vspace=\"0\"
hspace=\"0\" ");
@@ -162,6 +165,7 @@
}
+ //TODO by nick - dima - looks like duplicate code methods. can we add it to super
class?
public String getHideStyle(FacesContext context, UIComponent component) {
// TODO Auto-generated method stub
if (!(component.getParent() instanceof UIPanelMenu)) {
Modified:
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/PanelMenuRenderer.java
===================================================================
---
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/PanelMenuRenderer.java 2007-06-02
01:45:10 UTC (rev 987)
+++
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/PanelMenuRenderer.java 2007-06-03
20:36:52 UTC (rev 988)
@@ -41,16 +41,13 @@
import org.richfaces.component.panelmenu.UIPanelMenuItem;
import org.richfaces.renderkit.iconImages.PanelMenuIconSpacer;
-/**
- * @author hans
- *
- */
public class PanelMenuRenderer extends PanelMenuRendererBase {
/* (non-Javadoc)
* @see org.ajax4jsf.framework.renderer.RendererBase#getComponentClass()
*/
protected Class getComponentClass() {
+ //TODO by nick - dima - add concrete component class
return UIComponent.class;
}
@@ -132,6 +129,7 @@
.append("',parentId:'")
.append((String) child.getParent().getClientId(context))
.append("'},{type:" + (child instanceof UIPanelMenuItem ?
"\"item\"":"\"node\""))
+ //TODO by nick - dima - use "".equals(onopen)
.append(",onopen:"+(onopen.equals("") ?
"\"\"" : "\"" + onopen +
"\"")+",onclose:"+(onclose.equals("") ?
"\"\"" : "\"" + onclose + "\""))
.append(",event:\"" + event + "\"")
.append(",mode:\"" + mode + "\"")
@@ -248,6 +246,7 @@
}
private void addActionIfNeeded(FacesContext context,UIComponent child,StringBuffer
buffer){
+ //TODO by nick - dima - use CommandScriptBuilder
if (child instanceof UIPanelMenuItem){
if (child.getChildCount()>0){
buffer.append(",'panelMenuItemAction'");
Modified:
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/PanelMenuRendererBase.java
===================================================================
---
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/PanelMenuRendererBase.java 2007-06-02
01:45:10 UTC (rev 987)
+++
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/PanelMenuRendererBase.java 2007-06-03
20:36:52 UTC (rev 988)
@@ -59,6 +59,7 @@
StringBuffer buffer = new StringBuffer();
String src = getIconByType("custom",false,context,component);
int w = 16; //width(context);
+ //TODO by nick - dima - use
writer.startElement()/writer.endElement()/getUtils().writeAttribute() calls to output
image
for (int i=0;i<level;i++){
buffer.append("<td><img src=\"" + src + "\"
alt=\"\" hspace=\"0\" vspace=\"0\"
height=\""+w+"\" width=\""+w+"\"
style=\"display:block;\" /></td>\n");
}
@@ -73,6 +74,7 @@
return level;
}
parent = parent.getParent();
+ //TODO by nick - dima - inc level counter only if parent is panelMenu(Group)
component
level++;
}
return level;
@@ -99,6 +101,7 @@
color = (String) skin.getParameter(context,"panelmenu.itemBulletColor");
}
if(iconType != null && !iconType.equals("none")){
+ //TODO by nick - dima - why not call image classes by their names in doc. Eg. not
PanelMenuIconArrowUp, but PanelMenuChevronImage
if (iconType.equals("custom")){
source = getResource(PanelMenuIconSpacer.class.getName()).getUri(context, color);
} else if (iconType.equals("triangle")) {
@@ -120,6 +123,7 @@
} else if (iconType.equals("grid")) {
source = getResource(PanelMenuIconDots.class.getName()).getUri(context, color);
} else {
+ //TODO by nick - dima - TemplateContext is deprecated and shouldn't be used
source = (String)getUtils().encodeResourceURL(new
TemplateContext(this,context,component),iconType);
}
}
@@ -226,8 +230,10 @@
protected String getItemMode(UIComponent component) {
String parentExpandMode = findMenu(component).getExpandMode();
String parentMode = findMenu(component).getMode();
+ //TODO by nick - dima - always evaluates to false
if (null == parentMode && "".equals(parentMode))
parentMode = "server";
+ //TODO by nick - dima - always evaluates to false
if (null == parentExpandMode && "".equals(parentExpandMode))
parentExpandMode = "none";
String mode = "none";
Modified:
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/iconImages/PanelMenuIconArrow.java
===================================================================
---
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/iconImages/PanelMenuIconArrow.java 2007-06-02
01:45:10 UTC (rev 987)
+++
trunk/sandbox/panelmenu/src/main/java/org/richfaces/renderkit/iconImages/PanelMenuIconArrow.java 2007-06-03
20:36:52 UTC (rev 988)
@@ -37,6 +37,7 @@
public class PanelMenuIconArrow extends PanelMenuIconBasic {
public PanelMenuIconArrow() {
+ //TODO by nick - dima - move the code to super class constructor
setRenderer(new GifRenderer());
setLastModified(new Date(InternetResourceBuilder.getInstance().getStartTime()));
}
Modified:
trunk/sandbox/panelmenu/src/main/resources/org/richfaces/renderkit/html/scripts/panelMenu.js
===================================================================
---
trunk/sandbox/panelmenu/src/main/resources/org/richfaces/renderkit/html/scripts/panelMenu.js 2007-06-02
01:45:10 UTC (rev 987)
+++
trunk/sandbox/panelmenu/src/main/resources/org/richfaces/renderkit/html/scripts/panelMenu.js 2007-06-03
20:36:52 UTC (rev 988)
@@ -6,6 +6,7 @@
window.Richfaces = {};
}
+//TODO by nick - dima - better not to use global object to track panels
var PanelMenuStorage = new Object();
PanelMenu = Class.create();
@@ -183,6 +184,7 @@
if (this.action !="panelMenuItemAction"){
this.preTrigger(e);
var form = Event.findElement(e, "form");
+ //TODO by nick - dima - no need to create form
if(!form || typeof(form) == 'undefined' || !form.tagName ||
form.tagName.toLowerCase() != 'form'){
form = document.createElement('form');
form.setAttribute('method', 'post');
Modified: trunk/sandbox/panelmenu/src/main/templates/org/richfaces/htmlPanelMenu.jspx
===================================================================
--- trunk/sandbox/panelmenu/src/main/templates/org/richfaces/htmlPanelMenu.jspx 2007-06-02
01:45:10 UTC (rev 987)
+++ trunk/sandbox/panelmenu/src/main/templates/org/richfaces/htmlPanelMenu.jspx 2007-06-03
20:36:52 UTC (rev 988)
@@ -30,10 +30,12 @@
<tbody>
<vcp:body>
+ <!-- TODO by nick - dima - vcp:body content is fully ignored -->
<f:call name="renderChildren" />
</vcp:body>
<tr style="display:none">
<td>
+ <!-- TODO by nick - dima - no need for clientId here -->
<f:clientid var="clientId"/>
<f:call name="insertScript"/>
</td>
Modified:
trunk/sandbox/panelmenu/src/main/templates/org/richfaces/htmlPanelMenuGroup.jspx
===================================================================
---
trunk/sandbox/panelmenu/src/main/templates/org/richfaces/htmlPanelMenuGroup.jspx 2007-06-02
01:45:10 UTC (rev 987)
+++
trunk/sandbox/panelmenu/src/main/templates/org/richfaces/htmlPanelMenuGroup.jspx 2007-06-03
20:36:52 UTC (rev 988)
@@ -12,6 +12,7 @@
<f:clientid var="clientId"/>
+ <!-- TODO by nick - dima - use #{clientId}tdhide instead of tdhide#{clientId} -->
<tr id="tdhide#{clientId}" style="#{this:getHideStyle(context,
component)}" >
<td>
<jsp:scriptlet>
@@ -31,20 +32,24 @@
<f:call name="utils.encodeAttributes">
<f:parameter value="onclick,onmousedown,onmouseup,onmousemove" />
</f:call>
+ <!-- TODO by nick - dima - encodeAttributes & encodePassThru conflict -->
<f:call name="utils.encodePassThru" />
<tr>
+ <!-- TODO by nick - dima - just id="#{clientId}" -->
<f:call name="utils.encodeId" />
<f:call name="insertTDs" />
<td>
<f:call name="insertImage">
<f:parameter value="left" />
</f:call>
- </td>
+ </td>
+ <!-- TODO by nick - dima - item_content should be removed -->
<td style="width:100%" class="item_content"
id="icon#{clientId}" >
<input type="hidden" name="panelMenuState#{clientId}"
value="#{this:isOpened(context, component)}" >
- </input>
+ </input>
+ <!-- TODO by nick - dima - one input should be enough -->
<input type="hidden" name="panelMenuAction#{clientId}"
value="" >
</input>
@@ -61,6 +66,7 @@
</td>
</tr>
<vcp:body>
+ <!-- TODO by nick - dima - vcp:body content is fully ignored -->
<f:call name="renderChildren" />
</vcp:body>
</f:root>
\ No newline at end of file
Property changes on: trunk/sandbox-samples/panelmenu-sample
___________________________________________________________________
Name: svn:ignore
+ target