[richfaces-svn-commits] JBoss Rich Faces SVN: r15817 - in root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces: renderkit and 1 other directory.
richfaces-svn-commits at lists.jboss.org
richfaces-svn-commits at lists.jboss.org
Mon Nov 2 13:23:43 EST 2009
Author: nbelaevski
Date: 2009-11-02 13:23:42 -0500 (Mon, 02 Nov 2009)
New Revision: 15817
Modified:
root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/ColumnsIterator.java
root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/DataIterator.java
root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/FixedChildrenIterator.java
root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/IteratorBase.java
root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UISubTable.java
root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/AbstractRowsRenderer.java
root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/AbstractTableRenderer.java
root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/CellEncodeEvent.java
root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/ElementEncodeListener.java
root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/SimpleDataTableRendererBase.java
root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/TableHolder.java
Log:
Code review results for tables-ui
Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/ColumnsIterator.java
===================================================================
--- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/ColumnsIterator.java 2009-11-02 16:11:39 UTC (rev 15816)
+++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/ColumnsIterator.java 2009-11-02 18:23:42 UTC (rev 15817)
@@ -42,10 +42,13 @@
protected UIComponent nextItem(){
while (childrenIterator != null && childrenIterator.hasNext()) {
UIComponent child = childrenIterator.next();
- if(child instanceof javax.faces.component.UIColumn || child instanceof Column){
+ if (child instanceof javax.faces.component.UIColumn || child instanceof Column) {
return child;
}
}
+
+ //TODO nick - free childrenIterator field
+
return null;
}
Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/DataIterator.java
===================================================================
--- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/DataIterator.java 2009-11-02 16:11:39 UTC (rev 15816)
+++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/DataIterator.java 2009-11-02 18:23:42 UTC (rev 15817)
@@ -24,6 +24,7 @@
import javax.faces.component.UIComponent;
+//TODO nick - rename to include component name
class DataIterator extends IteratorBase <UIComponent> {
private Iterator<UIComponent> facetsIterator;
@@ -41,13 +42,17 @@
UIComponent nextColumn = null;
while (nextColumn == null && childrenIterator.hasNext()) {
UIComponent child = childrenIterator.next();
- if((child instanceof UIColumn) && child.isRendered()) {
+ //TODO nick - why non-rendered children are filtered?
+ //TODO nick - should be (child instanceof UIColumn || child instanceof Column)?
+ if ((child instanceof UIColumn) && child.isRendered()) {
nextColumn = child;
} else if(checkAjaxComponent(child)) {
nextColumn = child;
}
}
+ //TODO nick - free childrenIterator
+
// ???
while (nextColumn == null && facetsIterator.hasNext()) {
UIComponent component = facetsIterator.next();
@@ -56,6 +61,8 @@
}
}
+ //TODO nick - free facetsIterator
+
return nextColumn;
}
@@ -67,6 +74,7 @@
return this.childrenIterator;
}
+ //TODO nick - what's this for?
protected boolean checkAjaxComponent(UIComponent child) {
return false;
}
Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/FixedChildrenIterator.java
===================================================================
--- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/FixedChildrenIterator.java 2009-11-02 16:11:39 UTC (rev 15816)
+++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/FixedChildrenIterator.java 2009-11-02 18:23:42 UTC (rev 15817)
@@ -63,6 +63,7 @@
protected UIComponent getNextFacet() {
Iterator<UIComponent> facetsIterator = getFacetsIterator();
+ //TODO nick - while -> if
while(facetsIterator.hasNext()) {
return facetsIterator.next();
}
Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/IteratorBase.java
===================================================================
--- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/IteratorBase.java 2009-11-02 16:11:39 UTC (rev 15816)
+++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/IteratorBase.java 2009-11-02 18:23:42 UTC (rev 15817)
@@ -7,6 +7,7 @@
* @author Anton Belevich
* @author Nick Belaevski
*/
+//TODO nick - rename this class
public abstract class IteratorBase<E> implements Iterator<E> {
private boolean isCompleted = false;
Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UISubTable.java
===================================================================
--- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UISubTable.java 2009-11-02 16:11:39 UTC (rev 15816)
+++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UISubTable.java 2009-11-02 18:23:42 UTC (rev 15817)
@@ -63,11 +63,13 @@
public void setSortExpression(String sortExpression) {
// Do nothing - subtable is not sortable element;
+ //TODO nick - throw exception
}
@Override
public boolean getRendersChildren() {
+ //TODO nick - why "false"?
return false;
}
}
Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/AbstractRowsRenderer.java
===================================================================
--- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/AbstractRowsRenderer.java 2009-11-02 16:11:39 UTC (rev 15816)
+++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/AbstractRowsRenderer.java 2009-11-02 18:23:42 UTC (rev 15817)
@@ -35,6 +35,7 @@
import org.ajax4jsf.renderkit.RendererUtils.HTML;
import org.richfaces.component.UIDataAdaptor;
import org.richfaces.component.UIDataTable;
+import org.richfaces.component.util.HtmlUtil;
/**
* @author shura
@@ -43,6 +44,7 @@
public abstract class AbstractRowsRenderer extends RendererBase implements DataVisitor, ElementEncodeListener {
public static final String[][] TABLE_EVENT_ATTRS = {
+ //TODO nick - clarify new names for attributes
{"onclick","onRowClick"},
{"ondblclick","onRowDblClick"},
{"onmousemove","onRowMouseMove"},
@@ -69,10 +71,13 @@
protected String getCellStyleClasses(FacesContext context, UIComponent component) {
StringBuffer styleClass = new StringBuffer();
+
// Construct predefined classes
+ //TODO nick - use FacesContext#attributes
Map<String, Object> requestMap = context.getExternalContext().getRequestMap();
Object parentPredefined = requestMap.get(AbstractRowsRenderer.SKIN_CELL_CLASS_KEY);
if (null != parentPredefined) {
+ //TODO nick - use HtmlUtil.concatClasses(String ...);
styleClass.append(parentPredefined).append(" ");
} else {
styleClass.append(getCellSkinClass());
@@ -131,6 +136,7 @@
}
public void encodeChildren(FacesContext context, UIComponent component) throws IOException {
+ //TODO nick - move this down by classes hierarchy
if(component instanceof UIDataTable) {
encodeRows(context, (UIDataTable)component);
}
@@ -168,6 +174,7 @@
StringBuffer styleClass = new StringBuffer();
// Construct predefined classes
+ //TODO nick - use HtmlUtil.concatClasses(String ...);
if (null != parentPredefined) {
styleClass.append(parentPredefined).append(" ");
} else if (null != predefined) {
@@ -191,6 +198,7 @@
StringBuffer style = new StringBuffer();
// Construct predefined styles
+ //TODO nick - use HtmlUtil.concatStyles(String ...);
if (null != parentPredefined) {
style.append(parentPredefined).append(" ");
} else if (null != predefined) {
Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/AbstractTableRenderer.java
===================================================================
--- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/AbstractTableRenderer.java 2009-11-02 16:11:39 UTC (rev 15816)
+++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/AbstractTableRenderer.java 2009-11-02 18:23:42 UTC (rev 15817)
@@ -49,6 +49,7 @@
* @param attributeName - attribute name
* @return true if specified attribute should generate header on the table
*/
+ //TODO nick - rename this method
public boolean isHeaderFactoryColumnAttributePresent(UIDataTable table,String attributeName) {
Iterator<UIComponent> columns = table.columns();
boolean result = false;
@@ -74,6 +75,7 @@
return count;
}
+ //TODO nick - rename method
protected int calculateRowColumns(Iterator<UIComponent> col) {
int count = 0;
int currentLength = 0;
Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/CellEncodeEvent.java
===================================================================
--- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/CellEncodeEvent.java 2009-11-02 16:11:39 UTC (rev 15816)
+++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/CellEncodeEvent.java 2009-11-02 18:23:42 UTC (rev 15817)
@@ -5,6 +5,7 @@
import javax.faces.component.UIColumn;
import javax.faces.context.FacesContext;
+//TODO nick - can this be UIComponent instead of UIColumn?
public class CellEncodeEvent implements ElementEncodeEvent <UIColumn> {
private FacesContext context;
Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/ElementEncodeListener.java
===================================================================
--- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/ElementEncodeListener.java 2009-11-02 16:11:39 UTC (rev 15816)
+++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/ElementEncodeListener.java 2009-11-02 18:23:42 UTC (rev 15817)
@@ -10,16 +10,19 @@
// public void encodeFirstCell(CellEncodeEvent event)throws IOException;
+ //TODO nick - also encodeFooterCell?
public void encodeHeaderCell(CellEncodeEvent event) throws IOException;
public void encodeCell(CellEncodeEvent event) throws IOException;
-
+
+ //TODO nick - encodeFirstRow and encodeRowStart are not on the same abstraction level
public void encodeFirstRow(RowEncodeEvent event) throws IOException;
public void encodeRowStart(RowEncodeEvent event) throws IOException;
public void encodeRowEnd(RowEncodeEvent event) throws IOException;
-
+
+ //TODO nick - should be removed?
public void encodeSubTable(FacesContext context, UISubTable subTable) throws IOException;
}
Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/SimpleDataTableRendererBase.java
===================================================================
--- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/SimpleDataTableRendererBase.java 2009-11-02 16:11:39 UTC (rev 15816)
+++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/SimpleDataTableRendererBase.java 2009-11-02 18:23:42 UTC (rev 15817)
@@ -86,6 +86,9 @@
// Encode colgroup definition.
ResponseWriter writer = context.getResponseWriter();
+
+ //TODO nick - do we need this element if "columnsWidth" is absent?
+ //TODO nick - use constants from HTML class for attribute/element names
writer.startElement("colgroup", table);
int columns = getColumnsCount(table);
@@ -113,6 +116,7 @@
public void encodeCaption(FacesContext context, UIDataTable table) throws IOException {
UIComponent caption = table.getCaption();
+ //TODO nick - check for "rendered" attribute
if (caption == null) {
return;
}
@@ -125,6 +129,7 @@
writer.writeAttribute("class", captionClass, "captionClass");
String captionStyle = (String) table.getAttributes().get("captionStyle");
+ //TODO nick - check for empty string
if (captionStyle != null) {
writer.writeAttribute("style", captionStyle, "captionStyle");
}
@@ -150,6 +155,7 @@
UIComponent header = table.getHeader();
boolean isEncodeHeaders = isEncodeHeaders(table);
+ //TODO nick - check for "rendered" attribute
if (header != null || isEncodeHeaders) {
ResponseWriter writer = context.getResponseWriter();
@@ -160,6 +166,7 @@
if (header != null) {
encodeTableHeaderFacet(context, columns, writer, header,
"rich-table-header",
+ //TODO nick - rename classes!!!
"rich-table-header-continue",
"rich-table-headercell",
headerClass, "th");
@@ -191,12 +198,16 @@
if (columnFacetPresent) {
writer.startElement("tr", table);
encodeStyleClass(writer, null, "rich-table-subfooter", null, footerClass);
+ //TODO nick - use "th" instead of "td"?
+ //TODO nick - rename method "encodeHeaderFacets"
encodeHeaderFacets(context, writer, tableColumns, "rich-table-subfootercell", footerClass, "footer",
"td", columns);
writer.endElement("tr");
}
if (footer != null) {
+ //TODO nick - use "th" instead of "td"?
+ //TODO nick - rename method "encodeTableHeaderFacet"
encodeTableHeaderFacet(context, columns, writer, footer,
"rich-table-footer",
"rich-table-footer-continue",
@@ -227,7 +238,8 @@
}
if (isColgroup) {
- RowEncoder encoder = getRowEncoder(null);
+ //TODO nick - tableHolder is null
+ RowEncoder encoder = getRowEncoder(null);
encoder.setHeader(true);
encoder.encodeRows(context, footer);
} else {
Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/TableHolder.java
===================================================================
--- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/TableHolder.java 2009-11-02 16:11:39 UTC (rev 15816)
+++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/TableHolder.java 2009-11-02 18:23:42 UTC (rev 15817)
@@ -33,7 +33,10 @@
public class TableHolder {
private UIDataAdaptor component;
private int rowCounter;
+
+ //TODO nick - not used anyhow
private int gridRowCounter;
+
private String[] rowClasses;
private String[] columnsClasses;
@@ -82,6 +85,7 @@
return getRowClass(row);
}
+ //TODO nick - this seems to be removed
public String getRowClass(int row) {
String rowClass = null;
if(null != rowClasses){
More information about the richfaces-svn-commits
mailing list