Posted by
Adam Heath-2 on
URL: http://ofbiz.116.s1.nabble.com/Re-svn-commit-r928451-ofbiz-trunk-framework-widget-templates-htmlScreenMacroLibrary-ftl-tp1694238p1694276.html
Adrian Crum wrote:
> --- On Sun, 3/28/10, Adam Heath <
[hidden email]> wrote:
>> Adrian Crum wrote:
>>> --- On Sun, 3/28/10, Adam Heath <
[hidden email]>
>> wrote:
>>>>
[hidden email]
>>>> wrote:
>>>>> Author: adrianc
>>>>> Date: Sun Mar 28 16:48:22 2010
>>>>> New Revision: 928451
>>>>>
>>>>> URL:
http://svn.apache.org/viewvc?rev=928451&view=rev>>>>> Log:
>>>>> Simplified label html renderer macro.
>>>>>
>>>>> Modified:
>>>>>
>>>>
>> ofbiz/trunk/framework/widget/templates/htmlScreenMacroLibrary.ftl
>>>>> Modified:
>> ofbiz/trunk/framework/widget/templates/htmlScreenMacroLibrary.ftl
>>>>> URL:
http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/templates/htmlScreenMacroLibrary.ftl?rev=928451&r1=928450&r2=928451&view=diff>>>>>
>> ==============================================================================
>>>>> ---
>> ofbiz/trunk/framework/widget/templates/htmlScreenMacroLibrary.ftl
>>>> (original)
>>>>> +++
>> ofbiz/trunk/framework/widget/templates/htmlScreenMacroLibrary.ftl
>>>> Sun Mar 28 16:48:22 2010
>>>>> @@ -66,47 +66,10 @@ under the License.
>>>>> <#if
>> text?has_content>
>>>>> <#-- Label
>> is considered block
>>>> level element in screen widget. There is not
>> reason to
>>>> render text outside of any html element. Use of
>> style
>>>> element has set pattern and we'll use style
>>>>> to
>> determine
>>>> appropriate html element to use -->
>>>>> - <#if style?has_content>
>>>>> - <#if style=="h1">
>>>>> - <h1
>>>>> - <#elseif
>> style=="h2">
>>>>> - <h2
>>>>> - <#elseif
>> style=="h3">
>>>>> - <h3
>>>>> - <#elseif
>> style=="h4">
>>>>> - <h4
>>>>> - <#elseif
>> style=="h5">
>>>>> - <h5
>>>>> - <#elseif
>> style=="h6">
>>>>> - <h6
>>>>> - <#else>
>>>>> - <p
>> class="${style}"
>>>>> - </#if>
>>>>> + <#if
>>>> "h1~h2~h3~h4~h5~h6"?contains(style)>
>>>>> + <${style}<#if
>>>> id?has_content>
>>>>
>> id="${id}"</#if>>${text}</${style}>
>>>> And if style="arch1"?
>>> + <p<#if
>> style?has_content> class="${style}"</#if><#if
>> id?has_content>
>> id="${id}"</#if>>${text}</p>
>>
>> You aren't making any sense.
>>
>> Based on what I see this change doing, we'll end up with
>> with:
>>
>> <arch1
>>
>> as the tag name, when style="arch1".
>
> Unless you try it out on your local copy, you will just have to trust me that it works. :-)
That's not a useful response. "Just trust me" is not helpful. If you
know why something will work, and someone obvious doesn't(me), then
just go ahead and explain it. Doing a bunch of hand-waving just gives
the hand-waver a feeling of superiority, and makes the receiver feel
inferior.
However, upon further reflection, my initial example of wrongness was
not correct. Consider when style="h1~h2". The original code would do
<p class="h1~h2">, which could be matched in css as p."h1~h2" { }.
Now, with this change, the tag will be <h1~h2>, which is very wrong.
The question to ask yourself, is: are the widget systems, and their
internal helper files, supposed to be general purpose, or are they
just to be used by ofbiz? If the latter, then we can do whatever we
want with not checking various parameters, as we can just fix all bad
uses.
However, if the former is the correct answer, then we have to handle,
and allow, all possible cases. This change keeps that from occuring.