Author: nbelaevski Date: 2009-12-28 17:46:45 -0500 (Mon, 28 Dec 2009) New Revision: 16214 Modified: root/ui-sandbox/trunk/components/tables/api/src/main/java/org/richfaces/component/Column.java root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/Expandable.java root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UIColumn.java root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UIColumnGroup.java root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UIDataTable.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/component/UIToggleControl.java root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/html/HtmlToggleControl.java root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/state/DataTableState.java root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/state/SubTableState.java root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/event/ToggleEvent.java root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/event/ToggleListener.java root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/model/visual/SortingFilteringVisualModel.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/DataTableRowsRenderer.java root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/ToggleControlRendererBase.java Log: Code review results committed Modified: root/ui-sandbox/trunk/components/tables/api/src/main/java/org/richfaces/component/Column.java =================================================================== --- root/ui-sandbox/trunk/components/tables/api/src/main/java/org/richfaces/component/Column.java 2009-12-28 14:55:14 UTC (rev 16213) +++ root/ui-sandbox/trunk/components/tables/api/src/main/java/org/richfaces/component/Column.java 2009-12-28 22:46:45 UTC (rev 16214) @@ -37,8 +37,8 @@ * The column allows data sorting * @return */ + //TODO nick - deprecate this? public abstract boolean isSortable(); - public abstract void setSortable(boolean sortable); /** @@ -47,6 +47,7 @@ * EL-expression is evaluated on every data row * @param sortExpression */ + //TODO nick - use ValueExpression public abstract void setSortExpression(String sortExpression); public abstract String getSortExpression(); @@ -55,12 +56,17 @@ * * @param sortOrder */ + //TODO nick - use annotation from Swing? public abstract void setSortOrder(Ordering sortOrder); public abstract Ordering getSortOrder(); + + //TODO nick - "filterExpression" is missing + //TODO nick - "selfSorted" is missing public abstract void setFilterMethod(MethodExpression methodExpression); public abstract MethodExpression getFilterMethod(); + //TODO nick - make this java.lang.Object. Use case: Long or Enum used as filtering criteria public abstract void setFilterValue(String filterValue); public abstract String getFilterValue(); Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/Expandable.java =================================================================== --- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/Expandable.java 2009-12-28 14:55:14 UTC (rev 16213) +++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/Expandable.java 2009-12-28 22:46:45 UTC (rev 16214) @@ -4,8 +4,10 @@ import org.richfaces.event.ToggleListener; +//TODO nick - move to API public interface Expandable { + //TODO nick - setExpanded() public boolean isExpanded(); public void addToggleListener(ToggleListener listener); Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UIColumn.java =================================================================== --- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UIColumn.java 2009-12-28 14:55:14 UTC (rev 16213) +++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UIColumn.java 2009-12-28 22:46:45 UTC (rev 16214) @@ -14,6 +14,7 @@ public abstract void setColspan(int colspan); + //TODO nick - implement in the proper way public Ordering getSortOrder() { return null; } Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UIColumnGroup.java =================================================================== --- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UIColumnGroup.java 2009-12-28 14:55:14 UTC (rev 16213) +++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UIColumnGroup.java 2009-12-28 22:46:45 UTC (rev 16214) @@ -61,6 +61,7 @@ return; } + //TODO nick - implement in the proper way visitor.process(context, null, argument); } Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UIDataTable.java =================================================================== --- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UIDataTable.java 2009-12-28 14:55:14 UTC (rev 16213) +++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UIDataTable.java 2009-12-28 22:46:45 UTC (rev 16214) @@ -66,6 +66,7 @@ model = new DataTableVisualModelImpl(); if ((modelExpression != null) && (!modelExpression.isReadOnly(elContext))) { + //TODO nick - handle ELException modelExpression.setValue(elContext, model); } } @@ -76,7 +77,9 @@ if(model.getSortingFilteringVisualModel() == null) { model.setSortingFilteringVisualModel(initSortingFilteringVisualModel()); - } + } else { + //TODO nick - init SortingFilteringVisualModel from model + } } dataTableState.setVisualModel(model); @@ -121,6 +124,7 @@ return getFacet("caption"); } + //TODO nick - move methods that work with CSS classes into renderer public String getTableSkinClass() { return "rich-table"; } 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-12-28 14:55:14 UTC (rev 16213) +++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UISubTable.java 2009-12-28 22:46:45 UTC (rev 16214) @@ -141,6 +141,7 @@ public DataTableState getParentState() { DataTableState dataTableState = null; + //TODO nick - walk by parents until UIDataTable is found or no parent exists UIComponent component = getParent(); if (component instanceof UIDataTable) { UIDataTable dataTable = (UIDataTable) component; @@ -160,6 +161,7 @@ DataTableState parentState = getParentState(); if (parentState != null) { + //TODO nick - use getClientId(FacesContext) state = parentState.getChildState(getClientId()); } @@ -179,6 +181,7 @@ } public void setToggleListener(MethodExpression toggleExpression) { + //TODO nick - use addToggleListener() & MethodExpression wrapper in tag getStateHelper().put(PropertyKeys.toggleExpression, toggleExpression); } @@ -188,7 +191,10 @@ @Override public void queueEvent(FacesEvent event) { + //TODO nick - what if event comes from the nested subTable? if (event instanceof ToggleEvent) { + //TODO nick - why Ajax is processing at special phase? + //TODO nick - "immediate" support? if(getSwichType().equals(SWITCH_TYPE_AJAX)) { event.setPhaseId(PhaseId.INVOKE_APPLICATION); } else { @@ -218,6 +224,7 @@ UIToggleControl toggle = (UIToggleControl)toggleEvent.getSource(); // toggle.addExecuteId((String)getAttributes().get("clientId")); + //TODO nick - replace with getClientId(FacesContext) toggle.addRenderId((String)getAttributes().get("clientId")); } } @@ -244,6 +251,7 @@ return COMPONENT_FAMILY; } + //TODO nick - these methods are renderer-specific, should be moved to renderer-related code @Override public String getRowSkinClass() { return "rich-subtable-row"; Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UIToggleControl.java =================================================================== --- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UIToggleControl.java 2009-12-28 14:55:14 UTC (rev 16213) +++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/UIToggleControl.java 2009-12-28 22:46:45 UTC (rev 16214) @@ -63,6 +63,7 @@ } public void addExecuteId(String id){ + //TODO nick - if user has provided collection in bean it shouldn't be modified by the component - create a copy getExecute().add(id); } @@ -76,6 +77,7 @@ } public void addRenderId(String id){ + //TODO nick - if user has provided collection in bean it shouldn't be modified by the component - create a copy getRender().add(id); } Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/html/HtmlToggleControl.java =================================================================== --- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/html/HtmlToggleControl.java 2009-12-28 14:55:14 UTC (rev 16213) +++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/html/HtmlToggleControl.java 2009-12-28 22:46:45 UTC (rev 16214) @@ -2,5 +2,6 @@ import org.richfaces.component.UIToggleControl; +//TODO nick - behavior support public class HtmlToggleControl extends UIToggleControl { } Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/state/DataTableState.java =================================================================== --- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/state/DataTableState.java 2009-12-28 14:55:14 UTC (rev 16213) +++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/state/DataTableState.java 2009-12-28 22:46:45 UTC (rev 16214) @@ -47,9 +47,11 @@ return this.rows; } + //TODO nick - save/restore first/rows/visualModel public void restoreState(FacesContext context, Object state) { Object[] newState = (Object[]) state; if (newState != null) { + //TODO nick - do we need to support SubTableState that are StateHolder? subTableStates = (Map<String, SubTableState>) newState[0]; } } Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/state/SubTableState.java =================================================================== --- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/state/SubTableState.java 2009-12-28 14:55:14 UTC (rev 16213) +++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/component/state/SubTableState.java 2009-12-28 22:46:45 UTC (rev 16214) @@ -54,6 +54,7 @@ this.model = model; } + //TODO nick - do we need to support SubTableVisualModel that are StateHolder? public void restoreState(FacesContext context, Object state) { Object[] currentState = (Object[]) state; this.model = (SubTableVisualModel) currentState[0]; Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/event/ToggleEvent.java =================================================================== --- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/event/ToggleEvent.java 2009-12-28 14:55:14 UTC (rev 16213) +++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/event/ToggleEvent.java 2009-12-28 22:46:45 UTC (rev 16214) @@ -8,6 +8,7 @@ * @author Anton Belevich * */ +//TODO nick - move to api public class ToggleEvent extends FacesEvent { private static final long serialVersionUID = 2916560585918250885L; Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/event/ToggleListener.java =================================================================== --- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/event/ToggleListener.java 2009-12-28 14:55:14 UTC (rev 16213) +++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/event/ToggleListener.java 2009-12-28 22:46:45 UTC (rev 16214) @@ -6,6 +6,7 @@ * @author Anton Belevich * */ +//TODO nick - move to api public interface ToggleListener extends FacesListener { public void processToggle(ToggleEvent event); Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/model/visual/SortingFilteringVisualModel.java =================================================================== --- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/model/visual/SortingFilteringVisualModel.java 2009-12-28 14:55:14 UTC (rev 16213) +++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/model/visual/SortingFilteringVisualModel.java 2009-12-28 22:46:45 UTC (rev 16214) @@ -4,6 +4,7 @@ public interface SortingFilteringVisualModel { + //TODO nick - make this Enum public String getSortMode(); public void setSortMode(String sortMode); 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-12-28 14:55:14 UTC (rev 16213) +++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/AbstractTableRenderer.java 2009-12-28 22:46:45 UTC (rev 16214) @@ -32,6 +32,7 @@ boolean result = false; while (columns.hasNext() && !result) { UIComponent component = columns.next(); + //TODO nick - only rendered facets should be handled result = (component.isRendered() && null != component.getFacet(facetName)); } return result; @@ -232,6 +233,7 @@ } + //TODO nick - use org.richfaces.component.util.HtmlUtil.concatClasses(String...) protected String mergeStyleClasses(String classAttribibute, String skinClass, UIComponent component) { String styleClass = null; String resultClass = skinClass; Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/DataTableRowsRenderer.java =================================================================== --- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/DataTableRowsRenderer.java 2009-12-28 14:55:14 UTC (rev 16213) +++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/DataTableRowsRenderer.java 2009-12-28 22:46:45 UTC (rev 16214) @@ -129,6 +129,7 @@ context.getAttributes().put(key, value); } + //TODO nick - nested tables will not work correct protected void saveRowStyles(FacesContext context, String firstRowClass, String rowClass, String cellClass) { put(context, FIRST_ROW_CLASS_KEY, firstRowClass); put(context, ROW_CLASS_KEY, rowClass); Modified: root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/ToggleControlRendererBase.java =================================================================== --- root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/ToggleControlRendererBase.java 2009-12-28 14:55:14 UTC (rev 16213) +++ root/ui-sandbox/trunk/components/tables/ui/src/main/java/org/richfaces/renderkit/ToggleControlRendererBase.java 2009-12-28 22:46:45 UTC (rev 16214) @@ -80,6 +80,7 @@ UIToggleControl toggleControl = (UIToggleControl) component; UISubTable subTable = findComponent(context, toggleControl); + //TODO nick - remove the next line subTable.getAttributes().get("expanded"); if (subTable != null) { ResponseWriter writer = context.getResponseWriter(); @@ -131,6 +132,8 @@ writer.writeAttribute(eventName, script, null); } + //TODO nick - if image is empty, then no tag should be encoded + //TODO nick - "alt" attribute is required for <IMG> if (image != null && image.trim().length() > 0) { writer.writeAttribute(HTML.SRC_ATTRIBUTE, image, null); }