OFBiz
  1. OFBiz
  2. OFBIZ-68

Break up application templates with multiple screenlets

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: order
    • Labels:
      None

      Description

      There are currently 7 freemarker files in the order applications that have multiple screenlets in the same template file. They are:

      entry/cart/showcart.ftl
      entry/cart/showPromotionDetails.ftl
      entry/order/orderheader.ftl
      entry/poheader.ftl (may not be used)
      order/orderheader.ftl
      quote/ViewQuoteInfo.ftl
      request/ViewRequestInfo.ftl

      I would like these to be split up. Screenlets, when defined make up a good chunk of reusable code. When multiple screenlets inhabit the same template file, you either have to reuse all of the code or none if it. By splitting these up, the parts are more likely to be reused than the whole.

      Because the order application is the base for so many custom applications in addition to ecomerce and wholesale and salesrep, making it reusable will lower the maintenance those applications require. Please vote on this and I will submit patches. Thanks

      1. ViewRequest.patch
        28 kB
        Chris Howe
      2. cssChange.patch
        2 kB
        Chris Howe
      3. ViewQuote.patch
        29 kB
        Chris Howe
      4. OrderOrderHeader.patch
        116 kB
        Chris Howe
      5. OrderEntryOrderHeader.patch
        33 kB
        Chris Howe
      6. showPromotionDetails.patch
        20 kB
        Chris Howe

        Activity

        Hide
        Chris Howe added a comment -

        ViewRequest.patch

        splits up the ViewRequestInfo.ftl file into 3 freemarker templates
        also creates appropriate updates to RequestScreens.xml in the order application as well as the ecommerce application. Please review that this method is acceptable.

        Show
        Chris Howe added a comment - ViewRequest.patch splits up the ViewRequestInfo.ftl file into 3 freemarker templates also creates appropriate updates to RequestScreens.xml in the order application as well as the ecommerce application. Please review that this method is acceptable.
        Hide
        Jacopo Cappellato added a comment -

        Chris,

        your work is very good and I've committed it, with some changes, in rev. 7906.
        Please, review it and see if everything is ok: I cleaned up a bit of the original code (following your suggestions in the comments in the screen) and I've also fixed two small errors inyour patch (in the ordermgr/RequestScreens.xml in chunk at line 107).

        Before working at the entry and order screens (rather messed up and frequently modified) you could refactor the quote screen as well.

        Also, consider that tomorrow David will move the OFBiz codebase to the ASF server: if you'll start working on this on monday, maybe it's better.
        Again thanks

        Show
        Jacopo Cappellato added a comment - Chris, your work is very good and I've committed it, with some changes, in rev. 7906. Please, review it and see if everything is ok: I cleaned up a bit of the original code (following your suggestions in the comments in the screen) and I've also fixed two small errors inyour patch (in the ordermgr/RequestScreens.xml in chunk at line 107). Before working at the entry and order screens (rather messed up and frequently modified) you could refactor the quote screen as well. Also, consider that tomorrow David will move the OFBiz codebase to the ASF server: if you'll start working on this on monday, maybe it's better. Again thanks
        Hide
        Chris Howe added a comment -

        Jacopo,

        For the component://order/widget/ordermgr/OrderRequestScreens.xml#ViewRequestInfo Screen, you didn't retain the container or add a css style that would maintain the left half, right half like the Order Header does using tables...

        ie some screenlets on the left half and some screenlets on the right half.

        it just requires something like
        <container style="lefthalf">

        and then an addition to maincss.css and mainecom.css that defines the lefthalf and right half class in their respective proportions (50% with a bit of padding)

        Show
        Chris Howe added a comment - Jacopo, For the component://order/widget/ordermgr/OrderRequestScreens.xml#ViewRequestInfo Screen, you didn't retain the container or add a css style that would maintain the left half, right half like the Order Header does using tables... ie some screenlets on the left half and some screenlets on the right half. it just requires something like <container style="lefthalf"> and then an addition to maincss.css and mainecom.css that defines the lefthalf and right half class in their respective proportions (50% with a bit of padding)
        Hide
        Jacopo Cappellato added a comment -

        Hi Chris,

        could you please provide a patch for this? Thanks.

        Show
        Jacopo Cappellato added a comment - Hi Chris, could you please provide a patch for this? Thanks.
        Hide
        Chris Howe added a comment -

        cssChange.patch

        updates the ecomcss.css, maincss.css, and order/widget/RequestScreen to allow a left half, right half and contained footer

        this took much longer to do than I thought it would. I hope IE7 final gets released soon, IE people use it and it actually does things correctly.grrr

        Show
        Chris Howe added a comment - cssChange.patch updates the ecomcss.css, maincss.css, and order/widget/RequestScreen to allow a left half, right half and contained footer this took much longer to do than I thought it would. I hope IE7 final gets released soon, IE people use it and it actually does things correctly.grrr
        Hide
        Jacopo Cappellato added a comment -

        Chris,

        thanks so much. They work very well.
        I've committed the patch for the order component (rev. 7921) and I've asked David if he could add the new styles you have defined.

        Show
        Jacopo Cappellato added a comment - Chris, thanks so much. They work very well. I've committed the patch for the order component (rev. 7921) and I've asked David if he could add the new styles you have defined.
        Hide
        Chris Howe added a comment -

        ViewQuote.patch

        follows the same approach as ViewRequest.patch.

        Note:
        OrderErrorQuoteNotFound and OrderErrorRequestNotFound are not in any property files

        Show
        Chris Howe added a comment - ViewQuote.patch follows the same approach as ViewRequest.patch. Note: OrderErrorQuoteNotFound and OrderErrorRequestNotFound are not in any property files
        Hide
        Jacopo Cappellato added a comment -

        Thanks Chris,

        your patch for quotes is in svn (with some small changes) with rev. 418557

        Show
        Jacopo Cappellato added a comment - Thanks Chris, your patch for quotes is in svn (with some small changes) with rev. 418557
        Hide
        Chris Howe added a comment -

        OrderOrderHeader.patch

        splits up order/webapp/ordermgr/order/orderheader.ftl and all screen definitions that call it

        I like this approach better concerning the orderitems and may want to go back and do it for the orderrequest and orderquote for both ecommerce and order

        Show
        Chris Howe added a comment - OrderOrderHeader.patch splits up order/webapp/ordermgr/order/orderheader.ftl and all screen definitions that call it I like this approach better concerning the orderitems and may want to go back and do it for the orderrequest and orderquote for both ecommerce and order
        Hide
        Chris Howe added a comment -

        OrderEntryOrderHeader.patch

        splits up order/webapp/ordermgr/entry/order/orderheader.ftl and all screen definitions that call it

        Show
        Chris Howe added a comment - OrderEntryOrderHeader.patch splits up order/webapp/ordermgr/entry/order/orderheader.ftl and all screen definitions that call it
        Hide
        Chris Howe added a comment -

        showPromotionDetails.patch

        splits up order/webapp/ordermgr/entry/cart/showPromotionDetails.ftl and all screen definitions that call it

        I'm not going to split up /entry/cart/showcart.ftl because it contains javascript functions at the begining that are not used in showcart.ftl. The functions are used in subsequent template files that are called after showcart.ftl. A consensus needs to be reached on how to best handle this as it is already kind of a mess.

        Show
        Chris Howe added a comment - showPromotionDetails.patch splits up order/webapp/ordermgr/entry/cart/showPromotionDetails.ftl and all screen definitions that call it I'm not going to split up /entry/cart/showcart.ftl because it contains javascript functions at the begining that are not used in showcart.ftl. The functions are used in subsequent template files that are called after showcart.ftl. A consensus needs to be reached on how to best handle this as it is already kind of a mess.
        Hide
        David E. Jones added a comment -

        Could I recommend closing this issue after the current patches are committed and moving to an easier model of one issue for each screen, or set of screens, that is split up? Ie, more or less one issue per patch so the status and commentary is easier to manage and understand.

        Show
        David E. Jones added a comment - Could I recommend closing this issue after the current patches are committed and moving to an easier model of one issue for each screen, or set of screens, that is split up? Ie, more or less one issue per patch so the status and commentary is easier to manage and understand.
        Hide
        Chris Howe added a comment -

        That should actually be the last patch for this issue anyway (provided it tests okay, which it should). In the future should I take advantage of the subtask feature?

        Show
        Chris Howe added a comment - That should actually be the last patch for this issue anyway (provided it tests okay, which it should). In the future should I take advantage of the subtask feature?
        Hide
        Scott Gray added a comment -

        I think this issue can be closed?

        Show
        Scott Gray added a comment - I think this issue can be closed?
        Hide
        Jacopo Cappellato added a comment -

        Chris,

        your last patches were no more applicable because some of the artifacts have been modified so I've manually applied them with some differences (because there were a few issues, such as some bad references to missing resources, probably custom templates); if you can, have a look at them to see if everything work as expected.

        Really thank for your work,

        Jacopo

        Show
        Jacopo Cappellato added a comment - Chris, your last patches were no more applicable because some of the artifacts have been modified so I've manually applied them with some differences (because there were a few issues, such as some bad references to missing resources, probably custom templates); if you can, have a look at them to see if everything work as expected. Really thank for your work, Jacopo

          People

          • Assignee:
            Jacopo Cappellato
            Reporter:
            Chris Howe
          • Votes:
            3 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development