Re: svn commit: r820232 - in /ofbiz/trunk/framework: common/webcommon/portal/showPortalPage.ftl widget/src/org/ofbiz/widget/WidgetWorker.java widget/src/org/ofbiz/widget/form/ModelForm.java widget/src/org/ofbiz/widget/screen/ScreenRenderer.java

[Scott Gray]

1. You don't need to use a counter in FreeMarker loops, there is a built-in variable provided 2. Having to know in advance that you are going to render a form more than once is far from an ideal solution, what is two different screens use the same form on the same page? Quick fixes do not belong in the framework.

3.  What about screens that use the screen cache?







Regards
Scott

On 30/09/2009, at 10:51 PM, hans...@apache.org wrote:

4. The ScreenRenderer class now has a field called renderFormSeqNumber even though the ScreenRenderer knows nothing about the form widget. In general I think this solution is messy and not well thought out, it concerns me because we already have the iterateId solution you provided the other day and I'm worried that we are going to end up with a whole stack of strange variables that make no sense to look at in the code. I would prefer to see a solution where the widgets or the screens or the context keeps track of what widgets have previously been rendered and have then automatically take care of uniquely identifying the form elements themselves.

Author: hansbak
Date: Wed Sep 30 09:51:01 2009
New Revision: 820232

URL: http://svn.apache.org/viewvc?rev=820232&view=rev
Log:


Modified:
   ofbiz/trunk/framework/common/webcommon/portal/showPortalPage.ftl
   ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java



URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/webcommon/portal/showPortalPage.ftl?rev=820232&r1=820231&r2=820232&view=diff

@@ -20,6 +20,7 @@
<#if portalPage?has_content>
<table width="100%">
  <tr>
+    <#assign renderSeq = 0/>
    <#list portalPageColumns?if_exists as portalPageColumn>

      <#assign firstInColumn = true/>
@@ -31,7 +32,9 @@

+            ${screens.setRenderFormUniqueSeq(renderSeq)}
            </div>
+            <#assign renderSeq = renderSeq+1/>
          </#if>
          <#assign firstInColumn = false/>
        </#if>


URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/WidgetWorker.java?rev=820232&r1=820231&r2=820232&view=diff

@@ -268,15 +268,19 @@
        ModelForm modelForm = modelFormField.getModelForm();
        Integer itemIndex = (Integer) context.get("itemIndex");
        String iterateId = "";
+        String formUniqueId = "";
        String formName = (String) context.get("formName");
        if (UtilValidate.isEmpty(formName)) {
            formName = modelForm.getName();
        }
        if (UtilValidate.isNotEmpty(context.get("iterateId"))) {
-               iterateId = (String) context.get("iterateId");
+            iterateId = (String) context.get("iterateId");
+        }
+        if (UtilValidate.isNotEmpty(context.get("formUniqueId"))) {
+            formUniqueId = (String) context.get("formUniqueId");
        }
        if (itemIndex != null) {

        } else {

        }


URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/form/ModelForm.java?rev=820232&r1=820231&r2=820232&view=diff

@@ -1423,6 +1423,10 @@



+                }
+
                this.resetBshInterpreter(localContext);




URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ScreenRenderer.java?rev=820232&r1=820231&r2=820232&view=diff

@@ -73,6 +73,7 @@
    protected Appendable writer;
    protected MapStack<String> context;
    protected ScreenStringRenderer screenStringRenderer;
+    protected int renderFormSeqNumber = 0;


        this.writer = writer;
@@ -129,10 +130,15 @@
                writer.append(gwo.toString());
            }
        } else {

        }
        return "";
    }
+
+    public void setRenderFormUniqueSeq (int renderFormSeqNumber) {
+        this.renderFormSeqNumber = renderFormSeqNumber;
+    }

    public ScreenStringRenderer getScreenStringRenderer() {
        return this.screenStringRenderer;

fix for OFBIZ-2414: if a portlet is included more than once the links do not work, this fix will make these links unique. Sponsored By Antwebsystems Co.Ltd: employee Berm ofbiz/trunk/framework/widget/src/org/ofbiz/widget/form/ ModelForm.java ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ ScreenRenderer.java Modified: ofbiz/trunk/framework/common/webcommon/portal/ showPortalPage.ftl = = = = = = = = ====================================================================== --- ofbiz/trunk/framework/common/webcommon/portal/showPortalPage.ftl (original) +++ ofbiz/trunk/framework/common/webcommon/portal/showPortalPage.ftl Wed Sep 30 09:51:01 2009 <td style="vertical-align: top; <#if portalPageColumn.columnWidthPercentage?has_content> width:$ {portalPageColumn.columnWidthPercentage}%;</#if>"> ${setRequestAttribute("portalPortletId", portlet.portalPortletId)} ${setRequestAttribute("portletSeqId", portlet.portletSeqId)} ${screens.render(portlet.screenLocation, portlet.screenName)} Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/ WidgetWorker.java = = = = = = = = ====================================================================== --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/ WidgetWorker.java (original) +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/ WidgetWorker.java Wed Sep 30 09:51:01 2009 - return formName + modelForm.getItemIndexSeparator() + itemIndex.intValue() + iterateId + modelForm.getItemIndexSeparator() + modelFormField.getName(); + return formName + modelForm.getItemIndexSeparator() + itemIndex.intValue() + iterateId + formUniqueId + modelForm.getItemIndexSeparator() + modelFormField.getName(); return formName + modelForm.getItemIndexSeparator() + modelFormField.getName(); Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/form/ ModelForm.java = = = = = = = = ====================================================================== --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/form/ ModelForm.java (original) +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/form/ ModelForm.java Wed Sep 30 09:51:01 2009 ModelFormAction.runSubActions(this.rowActions, localContext); localContext.put("itemIndex", Integer.valueOf(itemIndex - lowIndex)); + if (UtilValidate.isNotEmpty(context.get("renderFormSeqNumber"))) { + localContext.put("formUniqueId", "_"+context.get("renderFormSeqNumber")); if (Debug.verboseOn()) Debug.logVerbose("In form got another row, context is: " + localContext, module); Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ ScreenRenderer.java = = = = = = = = ====================================================================== --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ ScreenRenderer.java (original) +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ ScreenRenderer.java Wed Sep 30 09:51:01 2009 public ScreenRenderer(Appendable writer, MapStack<String> context, ScreenStringRenderer screenStringRenderer) { + context.put("renderFormSeqNumber", String.valueOf(renderFormSeqNumber)); modelScreen.renderScreenString(writer, context, screenStringRenderer);

smime.p7s
Description: S/MIME cryptographic signature