[ 
https://issues.apache.org/jira/browse/OFBIZ-3274?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12784888#action_12784888
 ] 

Adrian Crum commented on OFBIZ-3274:
------------------------------------

Bruno,

My previous comment about the GlobalDecorator means this: it should contain 
only the code needed by ALL screens. Much of the existing GlobalDecorator code, 
and some of the code in your patch adds things that don't belong there. I have 
complained about this before, but no one seems to care.

Ideally, the GlobalDecorator should have a handful of 
decorator-section-includes and nothing more. There shouldn't be complicated 
conditional logic in the GlobalDecorator, like: "if this variable exists, do 
this, otherwise do that..." - that is a bad design. In effect, the 
GlobalDecorator is trying to make decisions about the screens that use it. The 
individual screens should handle that. The GlobalDecorator should just render 
whatever is handed to it by the screens - it shouldn't care about what the 
screens contain.

I will give a couple of examples:

1. The GlobalDecorator has to determine if an application uses a menu widget or 
a Freemarker template for its application menu. Why does the GlobalDecorator 
need to know that? Those thirty lines of code could be replaced with

{code}
<decorator-section-include name="application-menu"/>
{code}

and each screen will be responsible for passing an application-menu section to 
the decorator. Applications are free to render their menus any way they want, 
and the GlobalDecorator doesn't need to know how they do it.

2. The GlobalDecorator has to determine if an application uses multiple columns 
in its layout. Why does the GlobalDecorator need to know that? A small 
percentage of screens use multiple column layouts, so why include that code in 
the GlobalDecorator? Multi-column screens should use a multi-column decorator 
that is then passed to the GlobalDecorator's body section.

I know these issues are beyond the scope of what you're trying to do here, but 
at the same time your patch contributes to the problem.



> Using decorator sections to control the left-bar
> ------------------------------------------------
>
>                 Key: OFBIZ-3274
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-3274
>             Project: OFBiz
>          Issue Type: Improvement
>          Components: framework
>    Affects Versions: SVN trunk
>            Reporter: Bruno Busco
>            Assignee: Bruno Busco
>         Attachments: OFBIZ-3274 DecoratorSectionLayout.patch, OFBIZ-3274 
> DecoratorSectionLayout.patch
>
>
> Hi,
> at the moment, in order to have a screen rendered with or without a left bar, 
> the variables "leftbarScreenName", "leftbarScreenLocation" and 
> "MainColumnStyle" need to be set to select a screen for the left bar and a 
> main column style.
> This must be done in the screen itself or an application decorator.
> With the attached patch, submitted for your review, a new GlobalDecorator 
> section named "left-bar" has been added. If a screen must be displayed with a 
> left bar this new decorator section needs to be filled with the selected 
> content.
> The main column style is defined in the Global decorator. In order to do this 
> a new screen condition has been added: "if-empty-decorator-section". This 
> condition allows to check if a decorator section has been added content or 
> not. (actually it only checks if the decorator section has been defined).
> In the patch I updated all catalog application screens to use this new method.
> If there are no problems with you with this, I will commit in the next days.
> Thank you for sharing your thoughts about.
> -Bruno

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to