Please review the attached patch fro the HtmlFormRenderer class
Locked
12
12
Please review the attached patch fro the HtmlFormRenderer class
 
|
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); } |
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
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); > } > |
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
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); >> } >> > > |
|
|
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 > >> > > > >> 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 > >> > > > >> 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 > >> > > > >> 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 > >> > > > >> 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); > >> } > >> > > > > > |
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
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); >>>> } >>>> >>> |
|
|
My only concern then is why the first change in the
patch _adds the align attribute (it's obvious the rest of them were already there). I think the "step in the right direction" is a fine approach and doesn't need to "fix" everything but it shouldn't throw another wrench into it. I'm not sure we have enough people to apply the patch (as opposed to simply reviewing it) to their development to be sure it's not throwing form widget uses out of alignment in use cases that use the areastyle to define their alignment. --- Jacopo Cappellato <[hidden email]> 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 > >>>> > > > > >>>> 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 > >>>> > > > > >>>> 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 > >>>> > > > > >>>> 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 > >>>> > > > > >>>> 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); > >>>> } > >>>> > >>> > > |
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
In reply to this post by Jacopo Cappellato
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); >>>>> } >>>>> >>>> >>>> > > |
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
In reply to this post by cjhowe
First of all, just to avoid confusion, the alignment in my patch was
wrong and the correct one would be: buffer.append("<th align=\"left\""); Without it the column headers (that are now by default left aligned because they are rendered with <td>) would be centered in all the generated lists. Jacopo Chris Howe wrote: > My only concern then is why the first change in the > patch _adds the align attribute (it's obvious the rest > of them were already there). I think the "step in the > right direction" is a fine approach and doesn't need > to "fix" everything but it shouldn't throw another > wrench into it. I'm not sure we have enough people to > apply the patch (as opposed to simply reviewing it) to > their development to be sure it's not throwing form > widget uses out of alignment in use cases that use the > areastyle to define their alignment. > > |
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
In reply to this post by Adrian Crum
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); } |
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
Nothing attached my friend.
-- Tim Ruppert HotWax Media http://www.hotwaxmedia.com o:801.649.6594 f:801.649.6595 On Jan 8, 2007, at 12:10 PM, 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 <[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#renderFormatHeaderRowCellO >>>> pen(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#renderFormatHeaderRowCellC >>>> lose(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#renderFormatHeaderRowFormC >>>> ellClose(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#renderFormatFieldRowTitleC >>>> ellOpen(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#renderFormatFieldRowTitleC >>>> ellClose(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#renderFormatHeaderRowCellClos > e(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#renderFormatHeaderRowFormCell > Close(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#renderFormatFieldRowTitleCell > Open(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#renderFormatFieldRowTitleCell > Close(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); > } > |
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
It was attached to my copy.
Tim Ruppert wrote: > Nothing attached my friend. > -- > Tim Ruppert > HotWax Media > http://www.hotwaxmedia.com > > o:801.649.6594 > f:801.649.6595 > > > On Jan 8, 2007, at 12:10 PM, 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 <[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#renderFormatHeaderRowCellO >>>>> pen(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#renderFormatHeaderRowCellC >>>>> lose(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#renderFormatHeaderRowFormC >>>>> ellClose(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#renderFormatFieldRowTitleC >>>>> ellOpen(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#renderFormatFieldRowTitleC >>>>> ellClose(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#renderFormatHeaderRowCellClos >> e(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#renderFormatHeaderRowFormCell >> Close(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#renderFormatFieldRowTitleCell >> Open(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#renderFormatFieldRowTitleCell >> Close(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); >> } >> > > |
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
Hmm - now that's something to figure out for sure. The attachment
certainly didn't come thru for me. I'm on Mac Mail - what client are you using? Cheers, Tim -- Tim Ruppert HotWax Media http://www.hotwaxmedia.com o:801.649.6594 f:801.649.6595 On Jan 8, 2007, at 12:17 PM, Adrian Crum wrote: > It was attached to my copy. > > > Tim Ruppert wrote: > >> Nothing attached my friend. >> -- >> Tim Ruppert >> HotWax Media >> http://www.hotwaxmedia.com >> o:801.649.6594 >> f:801.649.6595 >> On Jan 8, 2007, at 12:10 PM, 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 <[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#renderFormatHeaderRowCel >>>>>> lO pen(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#renderFormatHeaderRowCel >>>>>> lC lose(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#renderFormatHeaderRowFor >>>>>> mC ellClose(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#renderFormatFieldRowTitl >>>>>> eC ellOpen(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#renderFormatFieldRowTitl >>>>>> eC ellClose(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#renderFormatHeaderRowCellOp >>> en (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#renderFormatHeaderRowCellCl >>> os e(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#renderFormatHeaderRowFormCe >>> ll Close(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#renderFormatFieldRowTitleCe >>> ll Open(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#renderFormatFieldRowTitleCe >>> ll Close(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); >>> } >>> |
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
Mozilla.
Tim Ruppert wrote: > Hmm - now that's something to figure out for sure. The attachment > certainly didn't come thru for me. I'm on Mac Mail - what client are > you using? > > Cheers, > Tim > -- > Tim Ruppert > HotWax Media > http://www.hotwaxmedia.com > > o:801.649.6594 > f:801.649.6595 > > > On Jan 8, 2007, at 12:17 PM, Adrian Crum wrote: > >> It was attached to my copy. >> >> >> Tim Ruppert wrote: >> >>> Nothing attached my friend. >>> -- >>> Tim Ruppert >>> HotWax Media >>> http://www.hotwaxmedia.com >>> o:801.649.6594 >>> f:801.649.6595 >>> On Jan 8, 2007, at 12:10 PM, 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 <[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#renderFormatHeaderRowCel >>>>>>> lO pen(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#renderFormatHeaderRowCel >>>>>>> lC lose(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#renderFormatHeaderRowFor >>>>>>> mC ellClose(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#renderFormatFieldRowTitl >>>>>>> eC ellOpen(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#renderFormatFieldRowTitl >>>>>>> eC ellClose(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#renderFormatHeaderRowCellOp >>>> en (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#renderFormatHeaderRowCellCl >>>> os e(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#renderFormatHeaderRowFormCe >>>> ll Close(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#renderFormatFieldRowTitleCe >>>> ll Open(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#renderFormatFieldRowTitleCe >>>> ll Close(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); >>>> } >>>> > > |
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
In reply to this post by Tim Ruppert
Tim,
that's odd. Can you get the zipped version attached? If so, maybe your mail client is doing something funny with the taxt attachments... Jacopo Tim Ruppert wrote: > Hmm - now that's something to figure out for sure. The attachment > certainly didn't come thru for me. I'm on Mac Mail - what client are > you using? > > Cheers, > Tim > -- > Tim Ruppert > HotWax Media > http://www.hotwaxmedia.com > |
HtmlFormRenderer.patch.zip (1018 bytes)
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
Yep - I just got the zipped version - I wonder what the issue with
the .patch is? Thanks for checking, Jacopo and Adrian. Cheers, Tim -- Tim Ruppert HotWax Media http://www.hotwaxmedia.com o:801.649.6594 f:801.649.6595 On Jan 8, 2007, at 12:31 PM, Jacopo Cappellato wrote: > Tim, > > that's odd. > Can you get the zipped version attached? > If so, maybe your mail client is doing something funny with the > taxt attachments... > > Jacopo > > > Tim Ruppert wrote: >> Hmm - now that's something to figure out for sure. The >> attachment certainly didn't come thru for me. I'm on Mac Mail - >> what client are you using? >> Cheers, >> Tim >> -- >> Tim Ruppert >> HotWax Media >> http://www.hotwaxmedia.com > > <HtmlFormRenderer.patch.zip> |
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
In reply to this post by Jacopo Cappellato
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 <[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); > } > |
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
In reply to this post by Jacopo Cappellato
Jacopo,
I've attached a patch for the HtmlFormRenderer class that has your patch applied. I have also added comments on what changes need to be made to use css classes instead of hard-coded style properties. Search the patched file for "// css upgrade:" to see my comments. If my suggestestions seem okay, then I will create the necessary css classes and update the HtmlFormRenderer.java file. I know this is more than what you intended, but my thinking is to fix it all while we're at it. -Adrian 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 |
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
Oops, I forgot the patch.
I'll be glad when this Monday is over. *sigh* Adrian Crum wrote: > Jacopo, > > I've attached a patch for the HtmlFormRenderer class that has your patch > applied. I have also added comments on what changes need to be made to > use css classes instead of hard-coded style properties. Search the > patched file for "// css upgrade:" to see my comments. If my > suggestestions seem okay, then I will create the necessary css classes > and update the HtmlFormRenderer.java file. > > I know this is more than what you intended, but my thinking is to fix it > all while we're at it. > > -Adrian > > 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 > > > Index: framework/widget/src/org/ofbiz/widget/html/HtmlFormRenderer.java =================================================================== --- framework/widget/src/org/ofbiz/widget/html/HtmlFormRenderer.java (revision 494159) +++ framework/widget/src/org/ofbiz/widget/html/HtmlFormRenderer.java (working copy) @@ -1,12 +1,12 @@ /* * Copyright 2001-2006 The Apache Software Foundation - * + * * Licensed under the Apache License, Version 2.0 (the "License"); you may not * use this file except in compliance with the License. You may obtain a copy of * the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the @@ -66,7 +66,7 @@ public class HtmlFormRenderer implements FormStringRenderer { public static final String module = HtmlFormRenderer.class.getName(); - + HttpServletRequest request; HttpServletResponse response; protected String lastFieldGroupId = ""; @@ -107,6 +107,7 @@ buffer.append(tooltipStyle); buffer.append("\""); } + // css upgrade: enclosing chars should be defined in css class or omitted buffer.append("> -["); buffer.append(tooltip); buffer.append("]- </span>"); @@ -114,17 +115,17 @@ } public void addAstericks(StringBuffer buffer, Map context, ModelFormField modelFormField) { - + boolean requiredField = modelFormField.getRequiredField(); if (requiredField) { String requiredStyle = modelFormField.getRequiredFieldStyle(); - + if (UtilValidate.isEmpty(requiredStyle)) { buffer.append("*"); } } } - + /* (non-Javadoc) * @see org.ofbiz.widget.form.FormStringRenderer#renderDisplayField(java.lang.StringBuffer, java.util.Map, org.ofbiz.widget.form.ModelFormField.DisplayField) */ @@ -141,6 +142,7 @@ // add a style of red if this is a date/time field and redWhen is true if (modelFormField.shouldBeRed(context)) { + // css upgrade: text style should be defined in css class buffer.append(" style=\"color: red;\""); } @@ -151,7 +153,7 @@ if (displayField instanceof DisplayEntityField) { this.makeHyperlinkString(buffer, ((DisplayEntityField) displayField).getSubHyperlink(), context); } - + this.appendTooltip(buffer, context, modelFormField); this.appendWhitespace(buffer); @@ -211,6 +213,7 @@ // add a style of red if this is a date/time field and redWhen is true if (modelFormField.shouldBeRed(context)) { + // css upgrade: text style should be defined in css class buffer.append(" style=\"color: red;\""); } @@ -254,7 +257,7 @@ } buffer.append("/>"); - + this.addAstericks(buffer, context, modelFormField); this.makeHyperlinkString(buffer, textField.getSubHyperlink(), context); @@ -303,7 +306,7 @@ buffer.append("htmlEditArea"); buffer.append('"'); } - + buffer.append('>'); String value = modelFormField.getEntry(context, textareaField.getDefaultValue(context)); @@ -312,11 +315,11 @@ } buffer.append("</textarea>"); - + if (textareaField.getVisualEditorEnable()) { buffer.append("<script language=\"javascript\" src=\"/images/htmledit/whizzywig.js\" type=\"text/javascript\"></script>"); buffer.append("<script language=\"javascript\" type=\"text/javascript\"> buttonPath = \"/images/htmledit/\"; cssFile=\"/images/htmledit/simple.css\";makeWhizzyWig(\""); - if (UtilValidate.isNotEmpty(idName)) { + if (UtilValidate.isNotEmpty(idName)) { buffer.append(idName); } else { buffer.append("htmlEditArea"); @@ -345,7 +348,7 @@ ModelFormField modelFormField = dateTimeField.getModelFormField(); String paramName = modelFormField.getParameterName(context); String defaultDateTimeString = dateTimeField.getDefaultDateTimeString(context); - + Map uiLabelMap = (Map) context.get("uiLabelMap"); if (uiLabelMap == null) { Debug.logWarning("Could not find uiLabelMap in context", module); @@ -366,6 +369,7 @@ // add a style of red if this is a date/time field and redWhen is true if (modelFormField.shouldBeRed(context)) { + // css upgrade: text style should be defined in css class buffer.append(" style=\"color: red;\""); } @@ -399,7 +403,7 @@ buffer.append(" title=\""); buffer.append(localizedInputTitle); buffer.append('"'); - + String value = modelFormField.getEntry(context, dateTimeField.getDefaultValue(context)); if (UtilValidate.isNotEmpty(value)) { if(value.length() > maxlength) @@ -408,7 +412,7 @@ buffer.append(value); buffer.append('"'); } - + buffer.append(" size=\""); buffer.append(size); buffer.append('"'); @@ -448,18 +452,19 @@ buffer.append(",'"); buffer.append(UtilHttp.encodeBlanks(modelFormField.getEntry(context, defaultDateTimeString))); buffer.append("');\">"); - + buffer.append("<img src=\""); this.appendContentUrl(buffer, "/images/cal.gif"); + // css upgrade: dimensions should be defined in css class buffer.append("\" width=\"16\" height=\"16\" border=\"0\" alt=\""); buffer.append(localizedIconTitle); buffer.append("\" title=\""); buffer.append(localizedIconTitle); buffer.append("\"/></a>"); } - + // if we have an input method of time-dropdown, then render two dropdowns - if ("time-dropdown".equals(dateTimeField.getInputMethod())) { + if ("time-dropdown".equals(dateTimeField.getInputMethod())) { String classString = (className != null ? " class=\"" + className + "\" " : ""); boolean isTwelveHour = "12".equals(dateTimeField.getClock()); @@ -470,7 +475,7 @@ cal = Calendar.getInstance(); cal.setTime(defaultTimestamp); } catch (IllegalArgumentException e) { - Debug.logWarning("Form widget field [" + paramName + "] with input-method=\"time-dropdown\" was not able to understand the default time [" + Debug.logWarning("Form widget field [" + paramName + "] with input-method=\"time-dropdown\" was not able to understand the default time [" + defaultDateTimeString + "]. The parsing error was: " + e.getMessage(), module); } @@ -482,7 +487,7 @@ if (isTwelveHour) { for (int i = 1; i <= 12; i++) { buffer.append("<option value=\"").append(i).append("\""); - if (cal != null) { + if (cal != null) { int hour = cal.get(Calendar.HOUR_OF_DAY); if (hour == 0) hour = 12; if (hour > 12) hour -= 12; @@ -499,7 +504,7 @@ buffer.append(">").append(i).append("</option>"); } } - + // write the select for minutes buffer.append("</select>:<select name=\""); buffer.append(UtilHttp.makeCompositeParam(paramName, "minutes")).append("\""); @@ -564,7 +569,7 @@ buffer.append(idName); buffer.append('"'); } - + int otherFieldSize = dropDownField.getOtherFieldSize(); String otherFieldName = dropDownField.getParameterNameOther(context); if (otherFieldSize > 0) { @@ -574,7 +579,7 @@ buffer.append(modelForm.getName()); buffer.append("."); buffer.append(otherFieldName); - buffer.append(")\" "); + buffer.append(")\" "); /* */ } @@ -643,7 +648,7 @@ // Adapted from work by Yucca Korpela // http://www.cs.tut.fi/~jkorpela/forms/combo.html if (otherFieldSize > 0) { - + String fieldName = modelFormField.getParameterName(context); Map dataMap = modelFormField.getMap(context); if (dataMap == null) { @@ -651,7 +656,7 @@ } Object otherValueObj = dataMap.get(otherFieldName); String otherValue = (otherValueObj == null) ? "" : otherValueObj.toString(); - + buffer.append("<noscript>"); buffer.append("<input type='text' name='"); buffer.append(otherFieldName); @@ -701,9 +706,9 @@ ModelFormField modelFormField = checkField.getModelFormField(); // never used: ModelForm modelForm = modelFormField.getModelForm(); String currentValue = modelFormField.getEntry(context); - + buffer.append("<span"); - + String className = modelFormField.getWidgetStyle(); if (UtilValidate.isNotEmpty(className)) { buffer.append(" class=\""); @@ -782,7 +787,7 @@ buffer.append(action); buffer.append('"'); } - + buffer.append("/>"); buffer.append(optionValue.getDescription()); @@ -802,7 +807,7 @@ ModelForm modelForm = modelFormField.getModelForm(); String singleClickAction = " onClick=\"javascript:submitFormDisableButton(this)\" "; String event = null; - String action = null; + String action = null; if ("text-link".equals(submitField.getButtonType())) { buffer.append("<a"); @@ -845,7 +850,7 @@ buffer.append(" src=\""); this.appendContentUrl(buffer, submitField.getImageLocation()); buffer.append('"'); - + event = modelFormField.getEvent(); action = modelFormField.getAction(); if (UtilValidate.isNotEmpty(event) && UtilValidate.isNotEmpty(action)) { @@ -857,7 +862,7 @@ } else { buffer.append(singleClickAction); } - + buffer.append("/>"); } else { // default to "button" @@ -882,7 +887,7 @@ buffer.append('"'); } - + event = modelFormField.getEvent(); action = modelFormField.getAction(); if (UtilValidate.isNotEmpty(event) && UtilValidate.isNotEmpty(action)) { @@ -895,7 +900,7 @@ //add single click JS onclick buffer.append(singleClickAction); } - + buffer.append("/>"); } @@ -977,7 +982,7 @@ public void renderFieldTitle(StringBuffer buffer, Map context, ModelFormField modelFormField) { String tempTitleText = modelFormField.getTitle(context); String titleText = UtilHttp.encodeAmpersands(tempTitleText); - + if (UtilValidate.isNotEmpty(titleText)) { buffer.append("<span"); if (UtilValidate.isNotEmpty(modelFormField.getTitleStyle())) { @@ -986,7 +991,7 @@ buffer.append("\""); } buffer.append(">"); - renderHyperlinkTitle(buffer, context, modelFormField, titleText); + renderHyperlinkTitle(buffer, context, modelFormField, titleText); buffer.append("</span>"); this.appendWhitespace(buffer); @@ -1000,19 +1005,19 @@ boolean requiredField = modelFormField.getRequiredField(); if (requiredField) { buffer.append("<span"); - + String requiredStyle = modelFormField.getRequiredFieldStyle(); if (UtilValidate.isEmpty(requiredStyle)) { requiredStyle = modelFormField.getTitleStyle(); } - + if (UtilValidate.isNotEmpty(requiredStyle)) { buffer.append(" class=\""); buffer.append(requiredStyle); buffer.append("\""); } buffer.append(">"); - renderHyperlinkTitle(buffer, context, modelFormField, modelFormField.getTitle(context)); + renderHyperlinkTitle(buffer, context, modelFormField, modelFormField.getTitle(context)); buffer.append("</span>"); this.appendWhitespace(buffer); @@ -1081,26 +1086,26 @@ if (useRowSubmit) { buffer.append("<input type=\"hidden\" name=\"_useRowSubmit\" value=\"Y\"/>"); } - + ModelFormField submitField = modelForm.getMultiSubmitField(); if (submitField != null) { // Threw this in that as a hack to keep the submit button from expanding the first field // Needs a more rugged solution - // WARNING: this method (renderMultiFormClose) must be called after the + // WARNING: this method (renderMultiFormClose) must be called after the // table that contains the list has been closed (to avoid validation errors) so // we cannot call here the methods renderFormatItemRowCell*: for this reason // they are now commented. - + // this.renderFormatItemRowCellOpen(buffer, context, modelForm, submitField); // this.renderFormatItemRowCellClose(buffer, context, modelForm, submitField); - + // this.renderFormatItemRowCellOpen(buffer, context, modelForm, submitField); submitField.renderFieldString(buffer, context, this); // this.renderFormatItemRowCellClose(buffer, context, modelForm, submitField); - + } buffer.append("</form>"); @@ -1116,6 +1121,7 @@ buffer.append("\""); buffer.append(">"); } else { + // css upgrade: default table style should be defined in css class buffer.append("<table border=\"1\" cellpadding=\"2\" cellspacing=\"0\" class=\"calendarTable\">"); // DEJ 20050101 removed the width=\"100%\", doesn't look very good with CSS float: left based side "columns" } @@ -1170,7 +1176,8 @@ * @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"); + // css upgrade: default header style should be defined in css class + buffer.append("<th align=\"left\""); String areaStyle = modelFormField.getTitleAreaStyle(); if (UtilValidate.isNotEmpty(areaStyle)) { buffer.append(" class=\""); @@ -1185,12 +1192,13 @@ * @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\""); + // css upgrade: default header style should be defined in css class + buffer.append("<th align=\"center\""); String areaStyle = modelForm.getFormTitleAreaStyle(); if (UtilValidate.isNotEmpty(areaStyle)) { buffer.append(" class=\""); @@ -1205,7 +1213,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); } @@ -1214,7 +1222,7 @@ */ public void renderFormatHeaderRowFormCellTitleSeparator(StringBuffer buffer, Map context, ModelForm modelForm, ModelFormField modelFormField, boolean isLast) { buffer.append("<span"); - + String titleStyle = modelFormField.getTitleStyle(); if (UtilValidate.isNotEmpty(titleStyle)) { buffer.append(" class=\""); @@ -1234,11 +1242,11 @@ * @see org.ofbiz.widget.form.FormStringRenderer#renderFormatItemRowOpen(java.lang.StringBuffer, java.util.Map, org.ofbiz.widget.form.ModelForm) */ public void renderFormatItemRowOpen(StringBuffer buffer, Map context, ModelForm modelForm) { - Integer itemIndex = (Integer)context.get("itemIndex"); - + Integer itemIndex = (Integer)context.get("itemIndex"); + buffer.append("<tr"); if (itemIndex!=null) { - + if (itemIndex.intValue()%2==0) { String evenRowStyle = modelForm.getEvenRowStyle(); if (UtilValidate.isNotEmpty(evenRowStyle)) { @@ -1295,6 +1303,7 @@ * @see org.ofbiz.widget.form.FormStringRenderer#renderFormatItemRowFormCellOpen(java.lang.StringBuffer, java.util.Map, org.ofbiz.widget.form.ModelForm) */ public void renderFormatItemRowFormCellOpen(StringBuffer buffer, Map context, ModelForm modelForm) { + // css upgrade: default cell style should be defined in css class buffer.append("<td align=\"center\""); String areaStyle = modelForm.getFormWidgetAreaStyle(); if (UtilValidate.isNotEmpty(areaStyle)) { @@ -1315,6 +1324,7 @@ } public void renderFormatSingleWrapperOpen(StringBuffer buffer, Map context, ModelForm modelForm) { + // css upgrade: table style should be defined in css class buffer.append("<table border=\"0\" cellpadding=\"2\" cellspacing=\"0\">"); this.appendWhitespace(buffer); @@ -1348,7 +1358,8 @@ * @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\""); + // css upgrade: default title style should be defined in css class + buffer.append("<th width=\"20%\" align=\"right\""); String areaStyle = modelFormField.getTitleAreaStyle(); if (UtilValidate.isNotEmpty(areaStyle)) { buffer.append(" class=\""); @@ -1363,7 +1374,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); } @@ -1386,11 +1397,12 @@ } else { buffer.append("80"); } + // css upgrade: default cell style should be defined in css class buffer.append("%\" align=\"left\""); if (positionSpan > 0) { buffer.append(" colspan=\""); - // do a span of 1 for this column, plus 3 columns for each spanned - //position or each blank position that this will be filling in + // do a span of 1 for this column, plus 3 columns for each spanned + //position or each blank position that this will be filling in buffer.append(1 + (positionSpan * 3)); buffer.append("\""); } @@ -1441,7 +1453,7 @@ buffer.append("<option value=\"contains\"" + ("contains".equals(defaultOption)? " selected": "") + ">" + opContains + "</option>"); buffer.append("<option value=\"empty\"" + ("empty".equals(defaultOption)? " selected": "") + ">" + opIsEmpty + "</option>"); buffer.append("</select>"); - + buffer.append("<input type=\"text\""); String className = modelFormField.getWidgetStyle(); @@ -1453,6 +1465,7 @@ // add a style of red if this is a date/time field and redWhen is true if (modelFormField.shouldBeRed(context)) { + // css upgrade: text style should be defined in css class buffer.append(" style=\"color: red;\""); } @@ -1492,7 +1505,7 @@ buffer.append(modelFormField.getParameterName(context)); buffer.append("_ic\" value=\"Y\"" + (ignCase ? " checked=\"checked\"" : "") + "/>"); buffer.append(ignoreCase); - + buffer.append("</span>"); this.appendTooltip(buffer, context, modelFormField); @@ -1525,6 +1538,7 @@ // add a style of red if this is a date/time field and redWhen is true if (modelFormField.shouldBeRed(context)) { + // css upgrade: text style should be defined in css class buffer.append(" style=\"color: red;\""); } @@ -1583,6 +1597,7 @@ // add a style of red if this is a date/time field and redWhen is true if (modelFormField.shouldBeRed(context)) { + // css upgrade: text style should be defined in css class buffer.append(" style=\"color: red;\""); } @@ -1641,7 +1656,7 @@ String opEquals = UtilProperties.getMessage("conditional", "equals", locale); String opGreaterThan = UtilProperties.getMessage("conditional", "greater_than", locale); String opSameDay = UtilProperties.getMessage("conditional", "same_day", locale); - String opGreaterThanFromDayStart = UtilProperties.getMessage("conditional", + String opGreaterThanFromDayStart = UtilProperties.getMessage("conditional", "greater_than_from_day_start", locale); String opLessThan = UtilProperties.getMessage("conditional", "less_than", locale); String opUpToDay = UtilProperties.getMessage("conditional", "up_to_day", locale); @@ -1665,6 +1680,7 @@ // add a style of red if this is a date/time field and redWhen is true if (modelFormField.shouldBeRed(context)) { + // css upgrade: text style should be defined in css class buffer.append(" style=\"color: red;\""); } @@ -1717,14 +1733,14 @@ // search for a localized label for the icon if (uiLabelMap != null) { localizedIconTitle = (String) uiLabelMap.get("CommonViewCalendar"); - } + } // add calendar pop-up button and seed data IF this is not a "time" type date-find if (!"time".equals(dateFindField.getType())) { if ("date".equals(dateFindField.getType())) { buffer.append("<a href=\"javascript:call_cal_notime(document."); } else { - buffer.append("<a href=\"javascript:call_cal(document."); + buffer.append("<a href=\"javascript:call_cal(document."); } buffer.append(modelFormField.getModelForm().getCurrentFormName(context)); buffer.append('.'); @@ -1735,6 +1751,7 @@ buffer.append("<img src=\""); this.appendContentUrl(buffer, "/images/cal.gif"); + // css upgrade: dimensions should be defined in css class buffer.append("\" width=\"16\" height=\"16\" border=\"0\" alt=\""); buffer.append(localizedIconTitle); buffer.append("\" title=\""); @@ -1773,6 +1790,7 @@ // add a style of red if this is a date/time field and redWhen is true if (modelFormField.shouldBeRed(context)) { + // css upgrade: text style should be defined in css class buffer.append(" style=\"color: red;\""); } @@ -1808,7 +1826,7 @@ if ("date".equals(dateFindField.getType())) { buffer.append("<a href=\"javascript:call_cal_notime(document."); } else { - buffer.append("<a href=\"javascript:call_cal(document."); + buffer.append("<a href=\"javascript:call_cal(document."); } buffer.append(modelFormField.getModelForm().getCurrentFormName(context)); buffer.append('.'); @@ -1819,6 +1837,7 @@ buffer.append("<img src=\""); this.appendContentUrl(buffer, "/images/cal.gif"); + // css upgrade: dimensions should be defined in css class buffer.append("\" width=\"16\" height=\"16\" border=\"0\" alt=\""); buffer.append(localizedIconTitle); buffer.append("\" title=\""); @@ -1867,6 +1886,7 @@ // add a style of red if this is a date/time field and redWhen is true if (modelFormField.shouldBeRed(context)) { + // css upgrade: text style should be defined in css class buffer.append(" style=\"color: red;\""); } @@ -1895,7 +1915,7 @@ buffer.append("/>"); String descriptionFieldName = lookupField.getDescriptionFieldName(); - // add lookup pop-up button + // add lookup pop-up button if (UtilValidate.isNotEmpty(descriptionFieldName)) { buffer.append("<a href=\"javascript:call_fieldlookup3(document."); buffer.append(modelFormField.getModelForm().getCurrentFormName(context)); @@ -1929,6 +1949,7 @@ buffer.append(");\">"); buffer.append("<img src=\""); this.appendContentUrl(buffer, "/images/fieldlookup.gif"); + // css upgrade: dimensions should be defined in css class buffer.append("\" width=\"16\" height=\"16\" border=\"0\" alt=\"Lookup\"/></a>"); this.makeHyperlinkString(buffer, lookupField.getSubHyperlink(), context); @@ -1943,8 +1964,8 @@ targetService = "${targetService}"; } if (UtilValidate.isEmpty(targetService)) { - Debug.logWarning("TargetService is empty.", module); - return; + Debug.logWarning("TargetService is empty.", module); + return; } // get the parametrized pagination index and size fields @@ -1954,7 +1975,7 @@ int viewIndex = modelForm.getViewIndex(context); int viewSize = modelForm.getViewSize(context); int listSize = modelForm.getListSize(context); - + int lowIndex = modelForm.getLowIndex(context); int highIndex = modelForm.getHighIndex(context); int actualPageSize = modelForm.getActualPageSize(context); @@ -1985,8 +2006,10 @@ String paginateAnchor = modelForm.getPaginateTargetAnchor(); if (paginateAnchor != null) anchor = "#" + paginateAnchor; + // css upgrade: table style should be defined in css class buffer.append("<table border=\"0\" cellpadding=\"2\">\n"); buffer.append(" <tr>\n"); + // css upgrade: cell style should be defined in css class buffer.append(" <td align=\"right\">\n"); buffer.append(" <b>\n"); if (viewIndex > 0) { @@ -2044,6 +2067,7 @@ // add a style of red if this is a date/time field and redWhen is true if (modelFormField.shouldBeRed(context)) { + // css upgrade: text style should be defined in css class buffer.append(" style=\"color: red;\""); } @@ -2095,6 +2119,7 @@ // add a style of red if this is a date/time field and redWhen is true if (modelFormField.shouldBeRed(context)) { + // css upgrade: text style should be defined in css class buffer.append(" style=\"color: red;\""); } @@ -2191,9 +2216,9 @@ this.appendWhitespace(buffer); } - + public void renderFieldGroupOpen(StringBuffer buffer, Map context, ModelForm.FieldGroup fieldGroup) { - String style = fieldGroup.getStyle(); + String style = fieldGroup.getStyle(); if (UtilValidate.isNotEmpty(style)) { buffer.append("<div"); buffer.append(" class=\""); @@ -2201,24 +2226,26 @@ buffer.append("\">"); } } - + public void renderFieldGroupClose(StringBuffer buffer, Map context, ModelForm.FieldGroup fieldGroup) { - String style = fieldGroup.getStyle(); + String style = fieldGroup.getStyle(); if (UtilValidate.isNotEmpty(style)) { buffer.append("</div>"); } } - + public void renderBanner(StringBuffer buffer, Map context, ModelForm.Banner banner) { + // css upgrade: table style should be defined in css class buffer.append("<table width=\"100%\"><tr>"); String style = banner.getStyle(context); String leftStyle = banner.getLeftTextStyle(context); if (UtilValidate.isEmpty(leftStyle)) leftStyle = style; String rightStyle = banner.getRightTextStyle(context); if (UtilValidate.isEmpty(rightStyle)) rightStyle = style; - + String leftText = banner.getLeftText(context); if (UtilValidate.isNotEmpty(leftText)) { + // css upgrade: cell style should be defined in css class buffer.append("<td align=\"left\">"); if (UtilValidate.isNotEmpty(leftStyle)) { buffer.append("<div"); @@ -2233,9 +2260,10 @@ } buffer.append("</td>"); } - + String text = banner.getText(context); if (UtilValidate.isNotEmpty(text)) { + // css upgrade: cell style should be defined in css class buffer.append("<td align=\"center\">"); if (UtilValidate.isNotEmpty(style)) { buffer.append("<div"); @@ -2250,9 +2278,10 @@ } buffer.append("</td>"); } - + String rightText = banner.getRightText(context); if (UtilValidate.isNotEmpty(rightText)) { + // css upgrade: cell style should be defined in css class buffer.append("<td align=\"right\">"); if (UtilValidate.isNotEmpty(rightStyle)) { buffer.append("<div"); @@ -2269,7 +2298,7 @@ } buffer.append("</tr></table>"); } - + /** * Renders a link for the column header fields when there is a header-link="" specified in the <field > tag, using * style from header-link-style="". Also renders a selectAll checkbox in multi forms. @@ -2281,7 +2310,7 @@ public void renderHyperlinkTitle(StringBuffer buffer, Map context, ModelFormField modelFormField, String titleText) { if (UtilValidate.isNotEmpty(modelFormField.getHeaderLink())) { StringBuffer targetBuffer = new StringBuffer(); - FlexibleStringExpander target = new FlexibleStringExpander(modelFormField.getHeaderLink()); + FlexibleStringExpander target = new FlexibleStringExpander(modelFormField.getHeaderLink()); String fullTarget = target.expandString(context); targetBuffer.append(fullTarget); String targetType = HyperlinkField.DEFAULT_TARGET_TYPE; |
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
In reply to this post by Adrian Crum
Adrian, all,
first of all a very minor point: in your patch there are many lines changed just to suppress unnecessary blanks; this is fine but it could hide the real mods you did and make the review a bit more complex... I hope that this will not refrain the other committers, and most of all David Jones, that is the master of widgets :-) to read your patch and comment. Second, about the refactoring, here is what I think: 1) it's fine to move all the styling info (as you suggested in your comments) to the css 2) when we have to specify a non-standard style property (such as different align properties for the th elements discussed before) we should move them to a css class and hardcode the class name if one is not passed; for example: if (UtilValidate.isNotEmpty(titleStyle)) { buffer.append(" class=\""); buffer.append(titleStyle); buffer.append("\""); } should be changed to: if (UtilValidate.isNotEmpty(titleStyle)) { buffer.append(" class=\""); buffer.append(titleStyle); buffer.append("\""); } else { buffer.append(" class=\"defaultTitleStyle\""); } 3) as a general rule, if the html element should be rendered with the default OFBiz style, there is no need to specify the hardcoded class as done in #2 4) as a future step: we should consider to change the single form html from table-based to div-based Jacopo Adrian Crum wrote: > Jacopo, > > I've attached a patch for the HtmlFormRenderer class that has your patch > applied. I have also added comments on what changes need to be made to > use css classes instead of hard-coded style properties. Search the > patched file for "// css upgrade:" to see my comments. If my > suggestestions seem okay, then I will create the necessary css classes > and update the HtmlFormRenderer.java file. > > I know this is more than what you intended, but my thinking is to fix it > all while we're at it. > > -Adrian > > 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 |
Re: Please review the attached patch fro the HtmlFormRenderer class
|
|
Jacopo,
Thanks for taking a look at this! Comments inline: Jacopo Cappellato wrote: > in your patch there are many lines changed just to suppress unnecessary > blanks; this is fine but it could hide the real mods you did and make > the review a bit more complex... I hope that this will not refrain the > other committers, and most of all David Jones, that is the master of > widgets :-) to read your patch and comment. I'm not doing any reformatting manually - my editor must be doing that without my knowledge. I don't know how to make it stop. I apologize for any confusion it causes. One of the things I noticed about this java file is that it doesn't follow the current OFBiz formatting guidelines. I would like to reformat the entire file to bring it up to standard. What do you think? > 2) when we have to specify a non-standard style property (such as > different align properties for the th elements discussed before) we > should move them to a css class and hardcode the class name if one is > not passed; for example: > > if (UtilValidate.isNotEmpty(titleStyle)) { > buffer.append(" class=\""); > buffer.append(titleStyle); > buffer.append("\""); > } > > should be changed to: > > if (UtilValidate.isNotEmpty(titleStyle)) { > buffer.append(" class=\""); > buffer.append(titleStyle); > buffer.append("\""); > } else { > buffer.append(" class=\"defaultTitleStyle\""); > } That's exactly the approach I had in mind! I'll work on this today and get it submitted. |
«
Return to OFBiz - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |