[ 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.