ofbiz-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Adrian Crum <adri...@hlmksw.com>
Subject Re: Please review the attached patch fro the HtmlFormRenderer class
Date Mon, 08 Jan 2007 23:03:27 GMT
Jacopo,

I have applied your patch and I'm looking over it now. Something unrelated that 
we should fix right away is the "addAstericks" method. It should be renamed to 
"addAsterisks."


Jacopo Cappellato wrote:

> 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 <adrianc@hlmksw.com> 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);
>      }
>  

Mime
View raw message