Login  Register

Re: Please review the attached patch fro the HtmlFormRenderer class

Posted by Jacopo Cappellato on Jan 08, 2007; 7:10pm
URL: http://ofbiz.116.s1.nabble.com/Please-review-the-attached-patch-fro-the-HtmlFormRenderer-class-tp176227p176233.html

Adrian,

about combining the two files into one file, for me it would be fine.

I'd like to get a bit more feedback before applying this patch, but in
the meantime, if you want to test it, here is the 'correct' one (attached).

Thanks for your help.

Jacopo


Adrian Crum wrote:

> Jacopo,
>
> I would like to help with this. I could spend some time going through
> the HtmlFormRenderer.java file and testing the changes.
>
> Did you see the comments made earlier about combining the two css files
> into one file? If we could get that committed, then I could apply the
> necessary patches to the single file.
>
> -Adrian
>
> Jacopo Cappellato wrote:
>
>> Adrian, Chris,
>>
>> I agree with you that the align attributes should be removed.
>> However, since there were already many of them in that file, this
>> would require a bit more of work (that must be done but maybe at a
>> later task) on the css styles definition.
>>
>> Jacopo
>>
>>
>> Chris Howe wrote:
>>
>>> Won't the lines that follow the first change:
>>>
>>>         if (UtilValidate.isNotEmpty(areaStyle)) {
>>>             buffer.append(" class=\"");
>>>             buffer.append(areaStyle);
>>>             buffer.append("\"");
>>>         }
>>> handle any additional styling information, including
>>> text alignment (ie in css: text-align: right;)? And
>>> then locale specific css can be added to handle
>>> Adrian's concern. Does the th align=\"right\" simply
>>> provide a default alignment that the css will be able
>>> to override or will the attributes for the <th> tag
>>> take priority over css?  I forget the answer at the
>>> moment.
>>>
>>> --- Adrian Crum <[hidden email]> wrote:
>>>
>>>> Oops, ight-to-Left languages would want it
>>>> RIGHT-aligned.
>>>>
>>>> Adrian Crum wrote:
>>>>
>>>>> It would be nice if the 'align' property was
>>>>
>>>> removed too. Right-to-Left
>>>>
>>>>> languages would want it left-aligned.
>>>>>
>>>>>
>>>>> Jacopo Cappellato wrote:
>>>>>
>>>>>> Please review the attached patch fro the
>>>>
>>>> HtmlFormRenderer class:
>>>>
>>>>>> it simply changes the <td> elements to <th>
>>>>
>>>> elements when used in
>>>>
>>>>>> headers (for list based forms) and as field names
>>>>
>>>> for single forms.
>>>>
>>>>>> Can I commit it?
>>>>>>
>>>>>> Jacopo
>>>>>>
>>>>>>
>>>>>>
>>> ------------------------------------------------------------------------
>>>
>>>>>> Index:
>>>
>>> framework/widget/src/org/ofbiz/widget/html/HtmlFormRenderer.java
>>> ===================================================================
>>>
>>>>>> ---
>>>
>>> framework/widget/src/org/ofbiz/widget/html/HtmlFormRenderer.java
>>>
>>>>  
>>>>>> (revision 494101)
>>>>>> +++
>>>
>>> framework/widget/src/org/ofbiz/widget/html/HtmlFormRenderer.java
>>>
>>>>  
>>>>>> (working copy)
>>>>>> @@ -1170,7 +1170,7 @@
>>>>>>       * @see
>>>
>>> org.ofbiz.widget.form.FormStringRenderer#renderFormatHeaderRowCellOpen(java.lang.StringBuffer,
>>>
>>>
>>>>>> java.util.Map, org.ofbiz.widget.form.ModelForm,
>>>>>> org.ofbiz.widget.form.ModelFormField)
>>>>>>       */
>>>>>>      public void
>>>>
>>>> renderFormatHeaderRowCellOpen(StringBuffer buffer,
>>>>
>>>>>> Map context, ModelForm modelForm, ModelFormField
>>>>
>>>> modelFormField) {
>>>>
>>>>>> -        buffer.append("<td");
>>>>>> +        buffer.append("<th align=\"right\"");
>>>>>>          String areaStyle =
>>>>
>>>> modelFormField.getTitleAreaStyle();
>>>>
>>>>>>          if (UtilValidate.isNotEmpty(areaStyle))
>>>>
>>>> {
>>>>
>>>>>>              buffer.append(" class=\"");
>>>>>> @@ -1185,12 +1185,12 @@
>>>>>>       * @see
>>>
>>> org.ofbiz.widget.form.FormStringRenderer#renderFormatHeaderRowCellClose(java.lang.StringBuffer,
>>>
>>>
>>>>>> java.util.Map, org.ofbiz.widget.form.ModelForm,
>>>>>> org.ofbiz.widget.form.ModelFormField)
>>>>>>       */
>>>>>>      public void
>>>>
>>>> renderFormatHeaderRowCellClose(StringBuffer buffer,
>>>>
>>>>>> Map context, ModelForm modelForm, ModelFormField
>>>>
>>>> modelFormField) {
>>>>
>>>>>> -        buffer.append("</td>");
>>>>>> +        buffer.append("</th>");
>>>>>>          this.appendWhitespace(buffer);
>>>>>>      }
>>>>>>  
>>>>>>      public void
>>>>
>>>> renderFormatHeaderRowFormCellOpen(StringBuffer
>>>>
>>>>>> buffer, Map context, ModelForm modelForm) {
>>>>>> -        buffer.append("<td align=\"center\"");
>>>>>> +        buffer.append("<th align=\"center\"");
>>>>>>          String areaStyle =
>>>>
>>>> modelForm.getFormTitleAreaStyle();
>>>>
>>>>>>          if (UtilValidate.isNotEmpty(areaStyle))
>>>>
>>>> {
>>>>
>>>>>>              buffer.append(" class=\"");
>>>>>> @@ -1205,7 +1205,7 @@
>>>>>>       * @see
>>>
>>> org.ofbiz.widget.form.FormStringRenderer#renderFormatHeaderRowFormCellClose(java.lang.StringBuffer,
>>>
>>>
>>>>>> java.util.Map, org.ofbiz.widget.form.ModelForm)
>>>>>>       */
>>>>>>      public void
>>>>
>>>> renderFormatHeaderRowFormCellClose(StringBuffer
>>>>
>>>>>> buffer, Map context, ModelForm modelForm) {
>>>>>> -        buffer.append("</td>");
>>>>>> +        buffer.append("</th>");
>>>>>>          this.appendWhitespace(buffer);
>>>>>>      }
>>>>>>  
>>>>>> @@ -1348,7 +1348,7 @@
>>>>>>       * @see
>>>
>>> org.ofbiz.widget.form.FormStringRenderer#renderFormatFieldRowTitleCellOpen(java.lang.StringBuffer,
>>>
>>>
>>>>>> java.util.Map,
>>>>
>>>> org.ofbiz.widget.form.ModelFormField)
>>>>
>>>>>>       */
>>>>>>      public void
>>>>
>>>> renderFormatFieldRowTitleCellOpen(StringBuffer
>>>>
>>>>>> buffer, Map context, ModelFormField
>>>>
>>>> modelFormField) {
>>>>
>>>>>> -        buffer.append("<td width=\"20%\"
>>>>
>>>> align=\"right\"");
>>>>
>>>>>> +        buffer.append("<th width=\"20%\"
>>>>
>>>> align=\"right\"");
>>>>
>>>>>>          String areaStyle =
>>>>
>>>> modelFormField.getTitleAreaStyle();
>>>>
>>>>>>          if (UtilValidate.isNotEmpty(areaStyle))
>>>>
>>>> {
>>>>
>>>>>>              buffer.append(" class=\"");
>>>>>> @@ -1363,7 +1363,7 @@
>>>>>>       * @see
>>>
>>> org.ofbiz.widget.form.FormStringRenderer#renderFormatFieldRowTitleCellClose(java.lang.StringBuffer,
>>>
>>>
>>>>>> java.util.Map,
>>>>
>>>> org.ofbiz.widget.form.ModelFormField)
>>>>
>>>>>>       */
>>>>>>      public void
>>>>
>>>> renderFormatFieldRowTitleCellClose(StringBuffer
>>>>
>>>>>> buffer, Map context, ModelFormField
>>>>
>>>> modelFormField) {
>>>>
>>>>>> -        buffer.append("</td>");
>>>>>> +        buffer.append("</th>");
>>>>>>          this.appendWhitespace(buffer);
>>>>>>      }
>>>>>>  
>>>>>
>>>>>
>>
>>

Index: framework/widget/src/org/ofbiz/widget/html/HtmlFormRenderer.java
===================================================================
--- framework/widget/src/org/ofbiz/widget/html/HtmlFormRenderer.java (revision 494101)
+++ framework/widget/src/org/ofbiz/widget/html/HtmlFormRenderer.java (working copy)
@@ -1170,7 +1170,7 @@
      * @see org.ofbiz.widget.form.FormStringRenderer#renderFormatHeaderRowCellOpen(java.lang.StringBuffer, java.util.Map, org.ofbiz.widget.form.ModelForm, org.ofbiz.widget.form.ModelFormField)
      */
     public void renderFormatHeaderRowCellOpen(StringBuffer buffer, Map context, ModelForm modelForm, ModelFormField modelFormField) {
-        buffer.append("<td");
+        buffer.append("<th align=\"left\"");
         String areaStyle = modelFormField.getTitleAreaStyle();
         if (UtilValidate.isNotEmpty(areaStyle)) {
             buffer.append(" class=\"");
@@ -1185,12 +1185,12 @@
      * @see org.ofbiz.widget.form.FormStringRenderer#renderFormatHeaderRowCellClose(java.lang.StringBuffer, java.util.Map, org.ofbiz.widget.form.ModelForm, org.ofbiz.widget.form.ModelFormField)
      */
     public void renderFormatHeaderRowCellClose(StringBuffer buffer, Map context, ModelForm modelForm, ModelFormField modelFormField) {
-        buffer.append("</td>");
+        buffer.append("</th>");
         this.appendWhitespace(buffer);
     }
 
     public void renderFormatHeaderRowFormCellOpen(StringBuffer buffer, Map context, ModelForm modelForm) {
-        buffer.append("<td align=\"center\"");
+        buffer.append("<th align=\"center\"");
         String areaStyle = modelForm.getFormTitleAreaStyle();
         if (UtilValidate.isNotEmpty(areaStyle)) {
             buffer.append(" class=\"");
@@ -1205,7 +1205,7 @@
      * @see org.ofbiz.widget.form.FormStringRenderer#renderFormatHeaderRowFormCellClose(java.lang.StringBuffer, java.util.Map, org.ofbiz.widget.form.ModelForm)
      */
     public void renderFormatHeaderRowFormCellClose(StringBuffer buffer, Map context, ModelForm modelForm) {
-        buffer.append("</td>");
+        buffer.append("</th>");
         this.appendWhitespace(buffer);
     }
 
@@ -1348,7 +1348,7 @@
      * @see org.ofbiz.widget.form.FormStringRenderer#renderFormatFieldRowTitleCellOpen(java.lang.StringBuffer, java.util.Map, org.ofbiz.widget.form.ModelFormField)
      */
     public void renderFormatFieldRowTitleCellOpen(StringBuffer buffer, Map context, ModelFormField modelFormField) {
-        buffer.append("<td width=\"20%\" align=\"right\"");
+        buffer.append("<th width=\"20%\" align=\"right\"");
         String areaStyle = modelFormField.getTitleAreaStyle();
         if (UtilValidate.isNotEmpty(areaStyle)) {
             buffer.append(" class=\"");
@@ -1363,7 +1363,7 @@
      * @see org.ofbiz.widget.form.FormStringRenderer#renderFormatFieldRowTitleCellClose(java.lang.StringBuffer, java.util.Map, org.ofbiz.widget.form.ModelFormField)
      */
     public void renderFormatFieldRowTitleCellClose(StringBuffer buffer, Map context, ModelFormField modelFormField) {
-        buffer.append("</td>");
+        buffer.append("</th>");
         this.appendWhitespace(buffer);
     }