Author: nbelaevski Date: 2007-07-22 18:27:10 -0400 (Sun, 22 Jul 2007) New Revision: 1772 Added: trunk/sandbox/ui/message/design/ trunk/sandbox/ui/message/design/funcspec/ trunk/sandbox/ui/message/design/funcspec/FuncSpec - RF Message Component.doc trunk/sandbox/ui/message/design/funcspec/FuncSpec - RF Messages Component.doc Removed: trunk/sandbox/samples/tooltip-sample/ Modified: trunk/sandbox/samples/pom.xml trunk/sandbox/ui/message/src/main/java/org/richfaces/renderkit/html/RichMessageBaseRenderer.java Log: - Code review made for rich-message - Functional specification for rich-message commited - tooltip-sample removed Modified: trunk/sandbox/samples/pom.xml =================================================================== --- trunk/sandbox/samples/pom.xml 2007-07-22 12:17:52 UTC (rev 1771) +++ trunk/sandbox/samples/pom.xml 2007-07-22 22:27:10 UTC (rev 1772) @@ -18,7 +18,7 @@ <module>panel2-sample</module> <module>simpleTogglePanel2-sample</module> <module>panelmenu-sample</module> - <module>tooltip-sample</module> + <module>rich-message-demo </module> </modules> </project> \ No newline at end of file Added: trunk/sandbox/ui/message/design/funcspec/FuncSpec - RF Message Component.doc =================================================================== (Binary files differ) Property changes on: trunk/sandbox/ui/message/design/funcspec/FuncSpec - RF Message Component.doc ___________________________________________________________________ Name: svn:mime-type + application/octet-stream Added: trunk/sandbox/ui/message/design/funcspec/FuncSpec - RF Messages Component.doc =================================================================== (Binary files differ) Property changes on: trunk/sandbox/ui/message/design/funcspec/FuncSpec - RF Messages Component.doc ___________________________________________________________________ Name: svn:mime-type + application/octet-stream Modified: trunk/sandbox/ui/message/src/main/java/org/richfaces/renderkit/html/RichMessageBaseRenderer.java =================================================================== --- trunk/sandbox/ui/message/src/main/java/org/richfaces/renderkit/html/RichMessageBaseRenderer.java 2007-07-22 12:17:52 UTC (rev 1771) +++ trunk/sandbox/ui/message/src/main/java/org/richfaces/renderkit/html/RichMessageBaseRenderer.java 2007-07-22 22:27:10 UTC (rev 1772) @@ -18,6 +18,7 @@ import org.ajax4jsf.renderkit.HeaderResourcesRendererBase; import org.ajax4jsf.renderkit.RendererBase; import org.ajax4jsf.renderkit.RendererUtils.HTML; +import org.apache.commons.collections.CollectionUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.richfaces.component.UIRichMessage; @@ -35,6 +36,7 @@ public static final String COMPONENT_NOT_FOUND_IN_VIEW_WARN_MESSAGE = "component not found in the view WARNING"; + //TODO nick - anton - local fields are not allowed in renderers public boolean isError = false; private RendererBase messageTemplate = null; @@ -44,6 +46,7 @@ Iterator msgIter = null; + //TODO nick - anton - use || instead of && if(component == null && context == null){ throw new NullPointerException(NULL_PARAMETER_ERROR_MESSAGE); } @@ -64,8 +67,11 @@ String clientId = msgComponent.getClientId(context); String forClientId = msgComponent.getFor(); + //TODO nick - anton - stop encoding messages. we should ALWAYS encode root tag of the component + // to handle AJAX re-rendering even if messages are absent if(forClientId == null){ if(log.isInfoEnabled()){ + //TODO nick - anton - error or info? log.info(NULL_PARAMETER_ERROR_MESSAGE); } } @@ -74,7 +80,10 @@ ResponseWriter writer = context.getResponseWriter(); if(!msgIter.hasNext() && isError){ + //TODO nick - anton - that part of code fully ignores marker location + //TODO nick - anton - check getFacet(...) for null, that wouldn't cause creation of facets map. + // refer to JavaDoc for more if(msgComponent.getFacets().containsKey("passedMarker")){ UIComponent facet = msgComponent.getFacet("passedMarker"); @@ -82,15 +91,18 @@ String markerClass = (String) msgComponent.getAttributes().get("markerClass"); String markerStyle = (String) msgComponent.getAttributes().get("markerStyle"); + //TODO nick - anton - huge attributes list renderMarkerFacet(msgComponent, facet, context, writer, markerClass, markerStyle); } + //TODO nick - anton - store variable here and use it then if(msgComponent.getPassedLabel() != null){ String passLabel = (String) msgComponent.getPassedLabel(); String labelClass = (String) msgComponent.getAttributes().get("labelClass"); String labelStyle = (String) msgComponent.getAttributes().get("labelStyle"); + //TODO nick - anton - huge attributes list renderLabel(msgComponent, null, context, writer, labelClass, labelStyle, false, false, false,passLabel,true); } @@ -104,6 +116,7 @@ doEncodeEnd(writer, context, msgComponent); + //TODO nick - anton - too late to add component to re-rendering if(isError){ AjaxContext ajaxContext = AjaxContext.getCurrentInstance(context); @@ -122,9 +135,10 @@ boolean showDetail = uiMsg.isShowDetail(); + //TODO nick - anton - use getUtils().isBooleanAttribute(component, name) + // why not make it boolean? Object tooltip = uiMsg.getAttributes().get("tooltip"); boolean isTooltip = false; - if(tooltip instanceof Boolean){ isTooltip = ((Boolean)tooltip).booleanValue(); } @@ -218,10 +232,12 @@ protected void renderMarkerFacet(UIRichMessage uiMsg, UIComponent marker, FacesContext context, ResponseWriter writer, String markerClass, String markerStyle) throws IOException{ + //TODO nick - anton - check if marker is rendered if(marker == null){ return; } - + + //TODO nick - anton - default message element is rendered as span. Consider revising FuncSpec-RF-M-NFR-010-1.0 layout writer.startElement(HTML.td_ELEM, uiMsg); String classes = markerClass == null ? "rich-message-marker" : "rich-message-marker " + markerClass; @@ -292,18 +308,21 @@ }else{ UIComponent result = findForComponent(context, forClientId,component); if (result == null) { + //TODO nick - anton consider using CollectionUtils.EMPTY_COLLECTION.iterator() msgIter = Collections.EMPTY_LIST.iterator(); } else { msgIter = context.getMessages(result.getClientId(context)); } } }else{ + //TODO nick - anton - no need for global messages msgIter = context.getMessages(); } return msgIter; } + //TODO nick - anton - candidate for utility method protected UIComponent findForComponent(FacesContext context, String forClientId, UIComponent component){ if(null == forClientId || forClientId.length() == 0){ @@ -328,6 +347,7 @@ } } catch(Throwable t){ + //TODO nick - anton - do not catch Throwables!!! you can catch OutOfMemoryError and swallow it here throw new RuntimeException(COMPONENT_NOT_FOUND_IN_VIEW_WARN_MESSAGE + ":"+ forClientId); } @@ -343,6 +363,7 @@ protected UIComponent findComponentBelow(UIComponent startComponent, String forClientId){ UIComponent retComp = null; + //TODO nick - anton - better check if getChildCount() == 0 first List children = startComponent.getChildren(); for(int i = 0, size = children.size(); i < size; i++){ @@ -366,7 +387,8 @@ }; private RendererBase getMessageTemplate(){ - + //TODO nick - anton - really so no need for external template renderer + //TODO nick - anton - such code blocks should be somehow synchronized if (messageTemplate == null) { try { @@ -385,6 +407,7 @@ } protected Class getComponentClass(){ + //TODO nick - anton - return UIMessage.class here return getClass(); } }