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

[Anil Patel]

Scott,
Thanks for helping with keeping Ofbiz from becoming pile of hacks.

Begging is right word, nothing else seems to work. :(

Regards
Anil Patel

On Sep 30, 2009, at 11:34 PM, Scott Gray wrote:

Hi Hans,

That all well and good but what I have a problem with is short term solutions to long term problems, especially in the framework where things can be difficult to change further down the track. The widget code especially can be very difficult to navigate if things are allowed to get messy.

My personal preference would be that things remain broken until a solid solution is put into place but I haven't got the time to propose or create one so I guess we just have to put up with these hackish solutions until someone has the time to do it properly.

Even if you chose not to create a jira issue and patch for every single contribution I would beg with you to consider doing it for framework contributions and allow the community to have a say about changes to such an important piece of OFBiz.

Regards
Scott

On 1/10/2009, at 3:24 PM, Hans Bakker wrote:

Hi Scott,

The tip for the loop counter of freemarker is taken and we will change

it.

If you have a proposal to fix this error a better way sure we have no

problem accepting it. As it is now, it fixes a real problem because it

happens very frequently that portlets are called pretty often in the
same screen. By the way, that is good so we reuse code.

We are thinking about making text portlets for blocks of text where we can build normal websites from. This error however always prevented this

development.

Regards,
Hans


On Wed, 2009-09-30 at 23:47 +1300, Scott Gray wrote:

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?
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.

Regards
Scott

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

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

URL: http://svn.apache.org/viewvc?rev=820232&view=rev
Log:
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

Modified:
 ofbiz/trunk/framework/common/webcommon/portal/showPortalPage.ftl

ofbiz/trunk/framework/widget/src/org/ofbiz/widget/ WidgetWorker.java

 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
URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/framework/common/webcommon/portal/showPortalPage.ftl?rev=820232&r1=820231&r2=820232&view=diff
=
=
=
=
=
=
=
=

= = = =================================================================== --- ofbiz/trunk/framework/common/webcommon/portal/ showPortalPage.ftl

(original)

+++ ofbiz/trunk/framework/common/webcommon/portal/ showPortalPage.ftl

Wed Sep 30 09:51:01 2009
@@ -20,6 +20,7 @@
<#if portalPage?has_content>
<table width="100%">
<tr>
+    <#assign renderSeq = 0/>
  <#list portalPageColumns?if_exists as portalPageColumn>
    <td style="vertical-align: top; <#if
portalPageColumn.columnWidthPercentage?has_content> width:$
{portalPageColumn.columnWidthPercentage}%;</#if>">
    <#assign firstInColumn = true/>
@@ -31,7 +32,9 @@
          ${setRequestAttribute("portalPortletId",
portlet.portalPortletId)}
          ${setRequestAttribute("portletSeqId",
portlet.portletSeqId)}
          ${screens.render(portlet.screenLocation,
portlet.screenName)}
+            ${screens.setRenderFormUniqueSeq(renderSeq)}
          </div>
+            <#assign renderSeq = renderSeq+1/>
        </#if>
        <#assign firstInColumn = false/>
      </#if>

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

= = = ===================================================================

--- 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
@@ -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) {
-            return formName + modelForm.getItemIndexSeparator() +

itemIndex.intValue() + iterateId + modelForm.getItemIndexSeparator ()

+ modelFormField.getName();
+            return formName + modelForm.getItemIndexSeparator() +
itemIndex.intValue() + iterateId + formUniqueId +
modelForm.getItemIndexSeparator() + modelFormField.getName();
      } else {
          return formName + modelForm.getItemIndexSeparator() +
modelFormField.getName();
      }

Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/form/
ModelForm.java
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
=
=
=
=
=
=
=
=

= = = ===================================================================

--- 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
@@ -1423,6 +1423,10 @@
              ModelFormAction.runSubActions(this.rowActions,
localContext);

              localContext.put("itemIndex",
Integer.valueOf(itemIndex - lowIndex));
+                if
(UtilValidate.isNotEmpty(context.get("renderFormSeqNumber"))) {
+                    localContext.put("formUniqueId",
"_"+context.get("renderFormSeqNumber"));
+                }
+
              this.resetBshInterpreter(localContext);

              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
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
=
=
=
=
=
=
=
=

= = = ===================================================================

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

  public ScreenRenderer(Appendable writer, MapStack<String>
context, ScreenStringRenderer screenStringRenderer) {
      this.writer = writer;
@@ -129,10 +130,15 @@
              writer.append(gwo.toString());
          }
      } else {
+            context.put("renderFormSeqNumber",
String.valueOf(renderFormSeqNumber));
          modelScreen.renderScreenString(writer, context,
screenStringRenderer);
      }
      return "";
  }
+
+    public void setRenderFormUniqueSeq (int renderFormSeqNumber) {
+        this.renderFormSeqNumber = renderFormSeqNumber;
+    }

  public ScreenStringRenderer getScreenStringRenderer() {
      return this.screenStringRenderer;

--
Antwebsystems.com: Quality OFBiz services for competitive rates