[richfaces-svn-commits] JBoss Rich Faces SVN: r7779 - in trunk: framework/impl/src/main/java/org/richfaces/component and 2 other directories.

Author: nbelaevski Date: 2008-04-11 17:39:37 -0400 (Fri, 11 Apr 2008) New Revision: 7779 Modified: trunk/framework/impl/src/main/java/org/ajax4jsf/renderkit/AjaxRendererUtils.java trunk/framework/impl/src/main/java/org/richfaces/component/UISwitchablePanel.java trunk/ui/tabPanel/src/main/java/org/richfaces/component/UITab.java trunk/ui/tabPanel/src/main/java/org/richfaces/component/UITabPanel.java trunk/ui/tabPanel/src/main/java/org/richfaces/renderkit/TabHeaderRendererBase.java Log: Code review results for TabPanel committed Modified: trunk/framework/impl/src/main/java/org/ajax4jsf/renderkit/AjaxRendererUtils.java =================================================================== --- trunk/framework/impl/src/main/java/org/ajax4jsf/renderkit/AjaxRendererUtils.java 2008-04-11 21:36:29 UTC (rev 7778) +++ trunk/framework/impl/src/main/java/org/ajax4jsf/renderkit/AjaxRendererUtils.java 2008-04-11 21:39:37 UTC (rev 7779) @@ -32,7 +32,6 @@ import java.util.Map; import java.util.Set; -import javax.faces.component.ActionSource; import javax.faces.component.EditableValueHolder; import javax.faces.component.NamingContainer; import javax.faces.component.UIComponent; @@ -902,6 +901,8 @@ } /** + * TODO: add deprecation + * * @param facesContext * @param component * @param id Modified: trunk/framework/impl/src/main/java/org/richfaces/component/UISwitchablePanel.java =================================================================== --- trunk/framework/impl/src/main/java/org/richfaces/component/UISwitchablePanel.java 2008-04-11 21:36:29 UTC (rev 7778) +++ trunk/framework/impl/src/main/java/org/richfaces/component/UISwitchablePanel.java 2008-04-11 21:39:37 UTC (rev 7779) @@ -73,6 +73,7 @@ return (String) switchTypeBinding.getValue(FacesContext.getCurrentInstance()); } + //TODO use DEFAULT_METHOD return SERVER_METHOD; } @@ -100,11 +101,12 @@ public void broadcast(FacesEvent facesEvent) throws AbortProcessingException { if(facesEvent instanceof ActionEvent){ - if(isImmediate()){ + //TODO invoke action listener or remove it + if(isImmediate()){ FacesContext facesContext = FacesContext.getCurrentInstance(); facesContext.renderResponse(); } - } + } //TODO else here if (facesEvent instanceof SwitchablePanelSwitchEvent) { if (isRendered()) { @@ -115,12 +117,13 @@ switchEvent.getValue()); Object oldValue = getValue(); - if (oldValue == null && newValue != null || - oldValue != null && !oldValue.equals(newValue)) { + if ((oldValue == null && newValue != null) || + (oldValue != null && !oldValue.equals(newValue))) { queueEvent(new ValueChangeEvent(this, oldValue, newValue)); } + //TODO UIInput should update the model, not the switchable panel itself ValueBinding valueBinding = getValueBinding("value"); if (valueBinding != null) { valueBinding.setValue(facesContext, newValue); @@ -190,33 +193,33 @@ } public void processUpdates(FacesContext context) { - if (context == null) { - throw new NullPointerException("FacesContext is null!"); - } + if (context == null) { + throw new NullPointerException("FacesContext is null!"); + } - if (!isRendered()) { - return ; - } + if (!isRendered()) { + return ; + } - Iterator kids = getSwitchedFacetsAndChildren(); - while (kids.hasNext()) { - UIComponent kid = (UIComponent) kids.next(); - kid.processUpdates(context); - } + Iterator kids = getSwitchedFacetsAndChildren(); + while (kids.hasNext()) { + UIComponent kid = (UIComponent) kids.next(); + kid.processUpdates(context); + } - try { - updateModel(context); + try { + updateModel(context); - if (!isValid()) { - context.renderResponse(); - } - - } catch (RuntimeException e) { - context.renderResponse(); - throw e; - } - } + if (!isValid()) { + context.renderResponse(); + } + } catch (RuntimeException e) { + context.renderResponse(); + throw e; + } + } + public void processValidators(FacesContext context) { if (context == null) { throw new NullPointerException("FacesContext is null!"); Modified: trunk/ui/tabPanel/src/main/java/org/richfaces/component/UITab.java =================================================================== --- trunk/ui/tabPanel/src/main/java/org/richfaces/component/UITab.java 2008-04-11 21:36:29 UTC (rev 7778) +++ trunk/ui/tabPanel/src/main/java/org/richfaces/component/UITab.java 2008-04-11 21:39:37 UTC (rev 7779) @@ -88,6 +88,7 @@ /* (non-Javadoc) * @see org.ajax4jsf.framework.ajax.AjaxActionComponent#setupReRender() */ + //TODO remove protected void setupReRender() { super.setupReRender(); AjaxRendererUtils.addRegionByName(getFacesContext(), this, this.getId()); Modified: trunk/ui/tabPanel/src/main/java/org/richfaces/component/UITabPanel.java =================================================================== --- trunk/ui/tabPanel/src/main/java/org/richfaces/component/UITabPanel.java 2008-04-11 21:36:29 UTC (rev 7778) +++ trunk/ui/tabPanel/src/main/java/org/richfaces/component/UITabPanel.java 2008-04-11 21:39:37 UTC (rev 7779) @@ -57,15 +57,23 @@ public boolean evaluate(Object object) { UITab tab = (UITab) object; - return !tab.isDisabled() && (CLIENT_METHOD.equals(tab.getSwitchTypeOrDefault())) || - (renderedValue != null && renderedValue.equals(tab.getName())); + + if (tab.isDisabled()) { + return false; + } + + if (CLIENT_METHOD.equals(tab.getSwitchTypeOrDefault())) { + return true; + } + + return renderedValue != null && renderedValue.equals(tab.getName()); } }); } public boolean isImmediate() { - + //TODO reorganize method if(!super.isImmediate()){ if(processedTabImmediate){ return true; @@ -77,7 +85,7 @@ } - + //TODO JavaDoc public Object convertSwitchValue(UIComponent component, Object object) { if (object != null) { return object; @@ -129,21 +137,21 @@ private transient Object restoredRenderedValue; public void processDecodes(FacesContext context) { - if (context == null) { - throw new NullPointerException("FacesContext is null!"); - } + if (context == null) { + throw new NullPointerException("FacesContext is null!"); + } - if (!isRendered()) { - return ; - } + if (!isRendered()) { + return ; + } - //RF-1047 - retry to obtain restoredRenderedValue. Seam conversations are absent on 1st phase thus - //value is absent also - if (restoredRenderedValue == null) { - this.restoredRenderedValue = getRenderedValue(); - } + //RF-1047 - retry to obtain restoredRenderedValue. Seam conversations are absent on 1st phase thus + //value is absent also + if (restoredRenderedValue == null) { + this.restoredRenderedValue = getRenderedValue(); + } - super.processDecodes(context); + super.processDecodes(context); } public Object saveState(FacesContext context) { @@ -158,21 +166,22 @@ Object[] states = (Object[]) state; super.restoreState(context, states[0]); this.renderedValue = states[1]; + //TODO remove that this.restoredRenderedValue = getRenderedValue(); } private static final Predicate RENDERED_TAB_PREDICATE = new Predicate() { - public boolean evaluate(Object object) { - if (object instanceof UITab) { - UITab tab = (UITab) object; - - return tab.isRendered(); - } - - return false; - } - + public boolean evaluate(Object object) { + if (object instanceof UITab) { + UITab tab = (UITab) object; + + return tab.isRendered(); + } + + return false; + } + }; /** @@ -182,13 +191,15 @@ * @return Iterator */ public Iterator getRenderedTabs() { - if (getChildCount() > 0) { - return new FilterIterator(getChildren().iterator(), RENDERED_TAB_PREDICATE); - } else { - return CollectionUtils.EMPTY_COLLECTION.iterator(); - } + if (getChildCount() > 0) { + return new FilterIterator(getChildren().iterator(), RENDERED_TAB_PREDICATE); + } else { + return CollectionUtils.EMPTY_COLLECTION.iterator(); + } } + + //TODO remove switchType attribute /** * Get Tab selection behavior for panel - one of "client", "server", "ajax", "page". * @@ -232,6 +243,7 @@ return (UITab)component; } + //TODO use getTabs() iterator private UITab getTabWithName(Object tabName){ List children = getChildren(); @@ -249,6 +261,7 @@ if(event instanceof SwitchablePanelSwitchEvent && this.equals(event.getComponent())){ SwitchablePanelSwitchEvent switchEvent = (SwitchablePanelSwitchEvent)event; UITab tab = processedTab(switchEvent.getEventSource(),switchEvent.getValue()); + //Check if target Tab is immediate processedTabImmediate = tab.isImmediate(); } Modified: trunk/ui/tabPanel/src/main/java/org/richfaces/renderkit/TabHeaderRendererBase.java =================================================================== --- trunk/ui/tabPanel/src/main/java/org/richfaces/renderkit/TabHeaderRendererBase.java 2008-04-11 21:36:29 UTC (rev 7778) +++ trunk/ui/tabPanel/src/main/java/org/richfaces/renderkit/TabHeaderRendererBase.java 2008-04-11 21:39:37 UTC (rev 7779) @@ -36,7 +36,6 @@ import org.ajax4jsf.javascript.ScriptUtils; import org.ajax4jsf.renderkit.AjaxRendererUtils; import org.ajax4jsf.renderkit.RendererUtils.HTML; -import org.ajax4jsf.util.HtmlDimensions; import org.richfaces.component.UISwitchablePanel; import org.richfaces.component.UITab; import org.richfaces.component.UITabPanel; @@ -51,17 +50,21 @@ private static final String LABEL_SUFFIX = "_lbl"; - protected Class getComponentClass() { + protected Class<? extends UIComponent> getComponentClass() { return UITab.class; } // find and encode UIParameter's components + //TODO generify + //TODO move the code to utils public List encodeParams(FacesContext context, UITab component) throws IOException { UITab menuItem = component; List params = new ArrayList(); + //TODO use StringBuilder StringBuffer buff = new StringBuffer(); + //TODO use getChildCount() > 0 List children = menuItem.getChildren(); for (Iterator iterator = children.iterator(); iterator.hasNext();) { UIComponent child = (UIComponent) iterator.next(); @@ -79,8 +82,11 @@ buff.append("] = "); buff.append(ScriptUtils.toScript(value)); buff.append(";"); + + //TODO ??? params.add(buff.toString()); - + + buff.setLength(0); } } } @@ -105,13 +111,20 @@ String clientId = tab.getClientId(context); //String style = "position:relative; top:1px;" + (String) tab.getAttributes().get("style"); + + //TODO use CSS classes String defShift = tab.isActive() ? "position:relative; top:1px;" : "position:relative;"; String componentStyle = (String) tab.getAttributes().get("style"); String style = defShift + (componentStyle != null ? componentStyle : ""); + //TODO format code block properly if (!disabled) { if (clientSide) { - writer.writeAttribute(HTML.onclick_ATTRIBUTE, "if (RichFaces.onTabChange(event, '"+pane.getClientId(context)+"','"+clientId+"')) RichFaces.switchTab('" + pane.getClientId(context) + "','" + clientId + "','" + getUtils().formatValue(context, pane, tab.getName()) + "');", "switchScript"); + //TODO use StringBuilder + writer.writeAttribute(HTML.onclick_ATTRIBUTE, "if (RichFaces.onTabChange(event, '"+pane.getClientId(context)+"','"+ + clientId+"')) RichFaces.switchTab('" + + pane.getClientId(context) + "','" + clientId + "','" + + getUtils().formatValue(context, pane, tab.getName()) + "');", "switchScript"); } else { String activeCheck = "if (RichFaces.isTabActive('" + clientId + LABEL_SUFFIX + "')) return false;"; String eventCheck = " if (!RichFaces.onTabChange(event, '"+pane.getClientId(context)+"','"+clientId+"')) return false;"; @@ -125,10 +138,11 @@ StringBuffer buffer = new StringBuffer(); function.appendScript(buffer); + //TODO remove this.onclick = null buffer.append("; return false; this.onclick = null;"); String script = buffer.toString(); writer.writeAttribute(HTML.onclick_ATTRIBUTE, activeCheck + eventCheck + script, null); - } else { + } else /* TODO if server */ { StringBuffer script = new StringBuffer("var _formName = A4J.findForm(this).id; var _paramName = '" + clientId + "_server_submit'; var _params = new Object(); _params[_paramName] = _paramName; "); List params = encodeParams(context, tab); @@ -139,6 +153,7 @@ script.append("_JSFFormSubmit('"); script.append(clientId); script.append("', _formName, null, _params);"); + //TODO remove this.onclick = null script.append("this.onclick = null; _clearJSFFormParameters(_formName, null, [_paramName]);"); writer.writeAttribute(HTML.onclick_ATTRIBUTE, activeCheck + eventCheck + script.toString() @@ -150,8 +165,10 @@ String width = tab.getLabelWidth(); + //TODO move to class style += ";height : 100%; "; if (width != null) { + //TODO use qualifySize style += " width: " + getUtils().encodePctOrPx(width) + ";"; } @@ -160,6 +177,7 @@ } } + //TODO review public boolean getRendersChildren() { return true; } @@ -202,21 +220,23 @@ writer.writeAttribute(HTML.title_ATTRIBUTE, title, null); } + //TODO make "label" constant UIComponent facet = tab.getFacet("label"); - if (facet != null && facet.isRendered()) { - renderChild(context, facet); - } else { - String label = tab.getLabel(); + if (facet != null && facet.isRendered()) { + renderChild(context, facet); + } else { + String label = tab.getLabel(); - if (label == null || label.length() == 0) { - label = "&#160;"; - writer.write(label); - }else{ - writer.writeText(label,null); - } - - } + if (label == null || label.length() == 0) { + //TODO to constant + label = "&#160;"; + writer.write(label); + }else{ + writer.writeText(label,null); + } + + } } @@ -232,7 +252,6 @@ public String encodeHeaderSpacing(FacesContext context, UITab tab) throws IOException { UITabPanel pane = tab.getPane(); String headerSpacing = pane.getHeaderSpacing(); - Double decoded = HtmlDimensions.decode(headerSpacing); return "width: " + HtmlUtil.qualifySize(headerSpacing) + "; "; }