Author: maksimkaszynski Date: 2008-04-10 04:58:09 -0400 (Thu, 10 Apr 2008) New Revision: 7727 Modified: trunk/ui/inplaceSelect/src/main/java/org/richfaces/component/UIInplaceSelect.java trunk/ui/inplaceSelect/src/main/java/org/richfaces/renderkit/InplaceSelectBaseRenderer.java trunk/ui/inplaceSelect/src/main/templates/inplaceselect.jspx Log: code reviewed Modified: trunk/ui/inplaceSelect/src/main/java/org/richfaces/component/UIInplaceSelect.java =================================================================== --- trunk/ui/inplaceSelect/src/main/java/org/richfaces/component/UIInplaceSelect.java 2008-04-10 08:58:03 UTC (rev 7726) +++ trunk/ui/inplaceSelect/src/main/java/org/richfaces/component/UIInplaceSelect.java 2008-04-10 08:58:09 UTC (rev 7727) @@ -5,18 +5,23 @@ import javax.faces.component.UIComponent; import javax.faces.component.UISelectOne; import javax.faces.context.FacesContext; +import javax.faces.model.SelectItem; import org.ajax4jsf.util.SelectUtils; /** * UI implementation of ComboBox component + * TODO: description here * @author Anton Belevich * @since 3.2.0 * */ public class UIInplaceSelect extends UISelectOne{ + + //TODO: Abstract class? - public List getSelectItems(FacesContext context, UIComponent component) { + //TODO: remove + public List<SelectItem> getSelectItems(FacesContext context, UIComponent component) { return SelectUtils.getSelectItems(context, component); } } Modified: trunk/ui/inplaceSelect/src/main/java/org/richfaces/renderkit/InplaceSelectBaseRenderer.java =================================================================== --- trunk/ui/inplaceSelect/src/main/java/org/richfaces/renderkit/InplaceSelectBaseRenderer.java 2008-04-10 08:58:03 UTC (rev 7726) +++ trunk/ui/inplaceSelect/src/main/java/org/richfaces/renderkit/InplaceSelectBaseRenderer.java 2008-04-10 08:58:09 UTC (rev 7727) @@ -28,11 +28,12 @@ public class InplaceSelectBaseRenderer extends ComboBoxBaseRenderer{ private static Log logger = LogFactory.getLog(InplaceSelectBaseRenderer.class); + //TODO: move duplicate constants to superclass private static final String RICH_INPLACE_SELECT_CLASSES = "rich-inplace-select-item rich-inplace-select-font"; private static final String CONTROLS_FACET = "controls"; private static final String EMPTY_DEFAULT_LABEL = "&#160;&#160;&#160;"; - + //Check copyPaste @Override protected void doDecode(FacesContext context, UIComponent component) { UIInplaceSelect inplaceInput = null; @@ -67,6 +68,8 @@ } } + //TODO: base on super method: + //return component != null ? this.getComponentClass().isAssignableFrom(component.getClass()) : false; protected boolean isAcceptableComponent(UIComponent component) { if (component instanceof UIInplaceSelect) { return true; @@ -80,11 +83,13 @@ return null; } + //TODO generify List parentList = new ArrayList(); UIInplaceSelect inplaceSelect = (UIInplaceSelect)component; List <String> labels = new ArrayList<String>(); ResponseWriter writer = context.getResponseWriter(); List<SelectItem> selectItems = SelectUtils.getSelectItems(context, inplaceSelect); + //TODO use foreach if (!selectItems.isEmpty()) { for (Iterator<SelectItem> iterator = selectItems.iterator(); iterator.hasNext();) { SelectItem selectItem = iterator.next(); @@ -94,7 +99,8 @@ encodeSuggestion(writer, inplaceSelect, label, RICH_INPLACE_SELECT_CLASSES); - List childList = new ArrayList(); + //TODO: review + List childList = new ArrayList(2); //new Object[2] childList.add(label); childList.add(value); parentList.add(childList); @@ -122,8 +128,9 @@ return UIInplaceSelect.class; } + //TODO: review public String getSelectedItemLabel(FacesContext context, UIInplaceSelect component) { - String defaultLabel = null; + //TODO: submitted value should be converted before searching for label Object value = component.getSubmittedValue(); if (value == null) { value = component.getAttributes().get("value"); @@ -131,7 +138,8 @@ if (value == null) { return createDefaultLabel(component); } - defaultLabel = getItemLabel(context, component, value); + + String defaultLabel = getItemLabel(context, component, value); if (defaultLabel == null) { return createDefaultLabel(component); } @@ -140,19 +148,25 @@ protected String getItemLabel(FacesContext context, UIComponent component, Object value) { String itemLabel = null; + //TODO: SelectUtils.getSelectItems is called minimum twice during encode List<SelectItem> selectItems = SelectUtils.getSelectItems(context, component); if (!selectItems.isEmpty()) { for(SelectItem item : selectItems) { if (value.equals(item.getValue())) { itemLabel = item.getLabel(); + //TODO break } } } + + //TODO exception if item hasn't been found + return itemLabel; } protected String createDefaultLabel(UIComponent component) { String defaultLabel = (String) component.getAttributes().get("defaultLabel"); + //TODO use length() if (defaultLabel == null || defaultLabel.equals("")) { defaultLabel = EMPTY_DEFAULT_LABEL; } @@ -160,7 +174,8 @@ } protected boolean isEmptyDefaultLabel(String defaultLabel) { - if (defaultLabel != null && EMPTY_DEFAULT_LABEL.equals(defaultLabel)) { + //TODO review + if (defaultLabel != null && EMPTY_DEFAULT_LABEL.equals(defaultLabel)) { return true; } return false; Modified: trunk/ui/inplaceSelect/src/main/templates/inplaceselect.jspx =================================================================== --- trunk/ui/inplaceSelect/src/main/templates/inplaceselect.jspx 2008-04-10 08:58:03 UTC (rev 7726) +++ trunk/ui/inplaceSelect/src/main/templates/inplaceselect.jspx 2008-04-10 08:58:09 UTC (rev 7727) @@ -53,7 +53,7 @@ if (cancelIcon != null && cancelIcon.length() != 0 ) { variables.setVariable("cancelIcon", cancelIcon); } - + //TODO use EL exprs. String controlClass = (String)component.getAttributes().get("controlClass"); variables.setVariable("controlClass", controlClass); String controlHoveredClass = (String)component.getAttributes().get("controlHoveredClass"); @@ -78,6 +78,7 @@ <jsp:scriptlet> if (layout.equals("inline")) { </jsp:scriptlet> + <!-- TODO #{component.attributes['viewClass']} --> <span id="#{clientId}" class="rich-inplace-select rich-inplace-select-view" x:passThruWithExclusions="id,styleClass,class,style"> <jsp:scriptlet> } else { @@ -138,6 +139,7 @@ </div> <div id="listParent#{clientId}" class="rich-inplace-select-width-list" style="display: none; position : absolute; height : 100px; left : 0px; top: 13px"> <div class="rich-inplace-select-list-shadow"> + <!-- TODO welcome magic numbers! --> <table id="shadow#{clientId}" cellpadding="0" cellspacing="0" border="0" width="257" height="109"> <tr> <td class="rich-inplace-select-shadow-tl"> @@ -211,6 +213,7 @@ } else { writer.writeText(convertToString(variables.getVariable("fieldLabel")),null); } + //TODO !layout.equals("inline") if (layout.equals("block")) { </jsp:scriptlet> </div>