Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: SVN trunk
    • Component/s: None
    • Labels:
      None

      Description

      The attached patch introduces a way to let developers to use the templates defined in htmlMacroFormLibrary.ftl in the FTL files.
      This was discussed in the mailing list and it seems that we all agree that having this feature could be a good thing.
      In the patch I used the renderLookupField in the main.ftl file of the catalog application. By doing this the two lookups that are included in this ftl are now rendered similar to how they are from a form widget.
      This allows the Tomahawk theme to render these lookups in the supposed way (with the card icon).

      Doing this I changed the macro in the htmlMacroFormLibrary.ftl to have all parameters with a default value so that only the relevant ones must be specified in the FTL.
      A new template.ftl file has been added but this only includes the htmlMacroFormLibrary.ftl. I do not know if there is a better way to have the macro available in the FTL file.

      Please fill free to comment any of the choise I did so that we could then commit this and extend to other macros.
      I guess the next one I will address is the renderNextPrev so that all paginations will look the same in all forms/FTL screens.

      1. widgetMacrosInFtlImprove.patch.diff
        14 kB
        Bilgin Ibryam
      2. widgetMacrosInFtl.patch
        6 kB
        Bruno Busco
      3. widgetMacrosInFtl.patch
        12 kB
        Bruno Busco
      4. widgetMacrosInFtl.patch
        15 kB
        Bilgin Ibryam
      5. OFBIZ-3541 Using Widgets html form templates in FTL files.patch
        57 kB
        Jacques Le Roux
      6. OFBIZ-3541 Using Widgets html form templates in FTL files.patch
        62 kB
        Jacques Le Roux
      7. OFBIZ-3541 Using Widgets html form templates in FTL files.patch
        61 kB
        Bilgin Ibryam
      8. OFBIZ-3541 Using Widgets html form templates in FTL files.patch
        81 kB
        Jacques Le Roux

        Activity

        Hide
        Jacques Le Roux added a comment -

        Hi Bruno,

        Just reviewed (not tested) but looks good to me: simple and clean.

        Show
        Jacques Le Roux added a comment - Hi Bruno, Just reviewed (not tested) but looks good to me: simple and clean.
        Hide
        Bruno Busco added a comment -

        Patch updated.
        Now the Webtools EntityList FTL based screen uses the same renderPrevNext macro used in form based screens.
        There is still an issue with the commonDisplaying label. I do not know how to have the $ contained in the label to be rendered while using the label in a FTL file.
        If someone can help...

        Show
        Bruno Busco added a comment - Patch updated. Now the Webtools EntityList FTL based screen uses the same renderPrevNext macro used in form based screens. There is still an issue with the commonDisplaying label. I do not know how to have the $ contained in the label to be rendered while using the label in a FTL file. If someone can help...
        Hide
        Jacques Le Roux added a comment -

        Hi Bruno,

        2 things I thought about lookups

        • why not create macro like for currency, etc. instead of an import to be done for each call (this is also true for renderPrevNext)
        • in a WIP I changed all calls in FTL files (but for Workeffort which call other lookups inside itself) from lookup2 to lookupLayer. This is not a big deal as it's only a simple S/R on all files. But I'd like to avoid the conflicts we will get if we don't synchronize our efforts. At which date do you expect to commit a global work on this?
        Show
        Jacques Le Roux added a comment - Hi Bruno, 2 things I thought about lookups why not create macro like for currency, etc. instead of an import to be done for each call (this is also true for renderPrevNext) in a WIP I changed all calls in FTL files (but for Workeffort which call other lookups inside itself) from lookup2 to lookupLayer. This is not a big deal as it's only a simple S/R on all files. But I'd like to avoid the conflicts we will get if we don't synchronize our efforts. At which date do you expect to commit a global work on this?
        Hide
        Bruno Busco added a comment -

        Hi Jacques,
        what do you mean for "create a macro like for currency"? Could you give me some pointer to the pattern you suggest?
        For #2, this is good to know. We can hold this patch until you commit all your work. There is no hurry.

        Show
        Bruno Busco added a comment - Hi Jacques, what do you mean for "create a macro like for currency"? Could you give me some pointer to the pattern you suggest? For #2, this is good to know. We can hold this patch until you commit all your work. There is no hurry.
        Hide
        Bilgin Ibryam added a comment -

        Bruno, I tested your patch, good work, thanks for it.

        Also updated the patch, so we don't need to import the macros in each template.

        Bilgin

        Show
        Bilgin Ibryam added a comment - Bruno, I tested your patch, good work, thanks for it. Also updated the patch, so we don't need to import the macros in each template. Bilgin
        Hide
        Jacques Le Roux added a comment -

        Thanks BIlgin,

        What was the point 1, Bruno. Now simples global S/R and, hop là

        Show
        Jacques Le Roux added a comment - Thanks BIlgin, What was the point 1, Bruno. Now simples global S/R and, hop là
        Hide
        Bruno Busco added a comment -

        Thank you Bilgin,
        so, now Jacques how should we proceed?
        Can I commit the patch?

        Show
        Bruno Busco added a comment - Thank you Bilgin, so, now Jacques how should we proceed? Can I commit the patch?
        Hide
        Bruno Busco added a comment -

        Then we could progressively use it in the FTL files but for this I can wait for your work done.
        Are you OK with this?

        Show
        Bruno Busco added a comment - Then we could progressively use it in the FTL files but for this I can wait for your work done. Are you OK with this?
        Hide
        Jacques Le Roux added a comment -

        Bruno,

        Actually it's not an issue. I can revert all the changes I made in the FTL files and after you will have commited your patch I will apply the changes again but using the macro rather than the js snippet.

        Show
        Jacques Le Roux added a comment - Bruno, Actually it's not an issue. I can revert all the changes I made in the FTL files and after you will have commited your patch I will apply the changes again but using the macro rather than the js snippet.
        Hide
        Bruno Busco added a comment -

        Patch committed to trunk rev. 920115, 920116 and 920117.
        Thank you Bilgin and Jacques.

        Show
        Bruno Busco added a comment - Patch committed to trunk rev. 920115, 920116 and 920117. Thank you Bilgin and Jacques.
        Hide
        Bilgin Ibryam added a comment -

        Bruno,

        I made some improvements to your code in order to add autocompleter support in ftl lookups.
        To achieve this, I introduced the initial template file you created and set there all the default values needed by htmlFormMacroLibrary.ftl. This gives more possibilities to get the default values in case they are not fixed strings (like autocompleter url) and keeps the rendering ftl cleaner.

        I attached the patch here, in case you (or someone else) wants to check it, before committing. Next strep would be to replace all the lookup fields (around 70) in ftl files with new lookup macro.

        Bilgin

        Show
        Bilgin Ibryam added a comment - Bruno, I made some improvements to your code in order to add autocompleter support in ftl lookups. To achieve this, I introduced the initial template file you created and set there all the default values needed by htmlFormMacroLibrary.ftl. This gives more possibilities to get the default values in case they are not fixed strings (like autocompleter url) and keeps the rendering ftl cleaner. I attached the patch here, in case you (or someone else) wants to check it, before committing. Next strep would be to replace all the lookup fields (around 70) in ftl files with new lookup macro. Bilgin
        Hide
        Jacques Le Roux added a comment -

        This patch (OFBIZ-3541 Using Widgets html form templates in FTL files.patch) is not yet ready to be commited.

        Actually only handling is needed now and I began to do it. You can see what is nedded to be done at the beginning of the patch. From findEmployee.ftl to findInventoryEventPlan.ftl the work is done.

        So we need to:

        • remove the <input type="text" line, keeping the value= attribute if it's no empty
        • remove the <img src= line complety (including the remaining </a>)

        If nobody beat me on it I will continue, of course all help would be appreciated. Also I not quite sure that all changes have been done.

        In other words, this is a 1st pass to be revisited...

        Show
        Jacques Le Roux added a comment - This patch ( OFBIZ-3541 Using Widgets html form templates in FTL files.patch) is not yet ready to be commited. Actually only handling is needed now and I began to do it. You can see what is nedded to be done at the beginning of the patch. From findEmployee.ftl to findInventoryEventPlan.ftl the work is done. So we need to: remove the <input type="text" line, keeping the value= attribute if it's no empty remove the <img src= line complety (including the remaining </a>) If nobody beat me on it I will continue, of course all help would be appreciated. Also I not quite sure that all changes have been done. In other words, this is a 1st pass to be revisited...
        Hide
        Jacques Le Roux added a comment -

        Bilgin,

        I did not test nor applied your patch yet. But it seems straightworward to change thereafter (when the above patch will be complete) all the <@formrenderer.renderLookupField by <@htmlTemplate.lookupField

        Thanks for your help

        Show
        Jacques Le Roux added a comment - Bilgin, I did not test nor applied your patch yet. But it seems straightworward to change thereafter (when the above patch will be complete) all the <@formrenderer.renderLookupField by <@htmlTemplate.lookupField Thanks for your help
        Hide
        Jacques Le Roux added a comment -

        Note that the patch above needs the OFBIZ-3442 patch to be applied before because of the targets changes in controllers. At some point though the old targets should be removed (but we might consider to keep them to allow popup lookups in future?)

        Show
        Jacques Le Roux added a comment - Note that the patch above needs the OFBIZ-3442 patch to be applied before because of the targets changes in controllers. At some point though the old targets should be removed (but we might consider to keep them to allow popup lookups in future?)
        Hide
        Jacques Le Roux added a comment -

        I found a 1st issue in applications/order/webapp/ordermgr/entry/cart/showcart.ftl

        The product input button at top of the 3d screen of order creation (in Order Manager) is shared between 3 feature : quick add, lookup and add gift certif (last is buggy). Not sure how to handle this correctly yet.

        Show
        Jacques Le Roux added a comment - I found a 1st issue in applications/order/webapp/ordermgr/entry/cart/showcart.ftl The product input button at top of the 3d screen of order creation (in Order Manager) is shared between 3 feature : quick add, lookup and add gift certif (last is buggy). Not sure how to handle this correctly yet.
        Hide
        Jacques Le Roux added a comment -

        To accomodate in the meantime the issue just above needs to have a LookupSupplierProductPopup and a LookupProductPopup besides the default layered lookups. I hope there will not too much cases like this one (should not be, it's very specific)

        Show
        Jacques Le Roux added a comment - To accomodate in the meantime the issue just above needs to have a LookupSupplierProductPopup and a LookupProductPopup besides the default layered lookups. I hope there will not too much cases like this one (should not be, it's very specific)
        Hide
        Jacques Le Roux added a comment -

        I found another in of shared input field in appendorderitem.ftl (with quicklookup)
        Also there we have an use of <@ofbizContentUrl>/images/fieldlookup.gif</@ofbizContentUrl> but I think that this is now deprecated by the themes. Not quite sure at this stage though...

        Show
        Jacques Le Roux added a comment - I found another in of shared input field in appendorderitem.ftl (with quicklookup) Also there we have an use of <@ofbizContentUrl>/images/fieldlookup.gif</@ofbizContentUrl> but I think that this is now deprecated by the themes. Not quite sure at this stage though...
        Hide
        Jacques Le Roux added a comment -

        Complete patch with the 2 issues I found with shared input (I marked them fix FIXME).

        I would appreciate reviews

        Show
        Jacques Le Roux added a comment - Complete patch with the 2 issues I found with shared input (I marked them fix FIXME). I would appreciate reviews
        Hide
        Jacques Le Roux added a comment -

        Bilgin,

        Could you please update your patch because I have changed some things since then and unfortunately I forgot to apply your patch before, sorry :/

        Show
        Jacques Le Roux added a comment - Bilgin, Could you please update your patch because I have changed some things since then and unfortunately I forgot to apply your patch before, sorry :/
        Hide
        Jacques Le Roux added a comment -

        Actually the last patch is not complete. I remember now that I forgot some other cases since I only treated some of the call_fieldlookup2 cases.
        There are still some

        • call_fieldlookup2 (17)
        • call_fieldlookup3 (5)
        • call_fieldlookup (2)
        Show
        Jacques Le Roux added a comment - Actually the last patch is not complete. I remember now that I forgot some other cases since I only treated some of the call_fieldlookup2 cases. There are still some call_fieldlookup2 (17) call_fieldlookup3 (5) call_fieldlookup (2)
        Hide
        Jacques Le Roux added a comment -

        You have to be very careful with regexp, you always forget something

        Show
        Jacques Le Roux added a comment - You have to be very careful with regexp, you always forget something
        Hide
        Bilgin Ibryam added a comment -

        Jacques,

        In rev 930660 I committed my patch (I don't want it to get obsolete) and also updated yours (replaced ormrenderer.renderLookupField with htmlTemplate.lookupField)

        Also could you tell me why we need to replace targets in controllers? Can't we reuse existing requests? I can't see the related discussion about this change?

        Show
        Bilgin Ibryam added a comment - Jacques, In rev 930660 I committed my patch (I don't want it to get obsolete) and also updated yours (replaced ormrenderer.renderLookupField with htmlTemplate.lookupField) Also could you tell me why we need to replace targets in controllers? Can't we reuse existing requests? I can't see the related discussion about this change?
        Hide
        Jacques Le Roux added a comment -

        Hi BIlgin,

        IIRW (I did not check) it's only beause of the decorators names, could be more reasons see OFBIZ-3442. Anyway this was mostly related to the testing phase. We should be able to simplify this and other points as we already discussed by setting the defaults in the renderer (I repeat: layer and topleft). For the moment actually there are only 4 points remaining:

        • cross button in header which does not close before you click inside the layer, not a big deal for now as there is a workaroud, Sascha will quickly fix this later he said
        • the minize button (+ -) has a problem, we have an idea on how to fix that thanks to Ankit Jain
        • the lookups that call calendars should not be layered until we solve the issue with them. I think Sascha will be able to solve that when he will come back
        • 2 inputs fields in FTL are shared (see my comment above) and we will need to keep them as is as long as we don't find an answer for them. The only drawback is the button image in Tomahawk.

        So nothing is really blocking now and we should be ok to commit all soon. Thanks for commiting you patch and updating mine

        Show
        Jacques Le Roux added a comment - Hi BIlgin, IIRW (I did not check) it's only beause of the decorators names, could be more reasons see OFBIZ-3442 . Anyway this was mostly related to the testing phase. We should be able to simplify this and other points as we already discussed by setting the defaults in the renderer (I repeat: layer and topleft). For the moment actually there are only 4 points remaining: cross button in header which does not close before you click inside the layer, not a big deal for now as there is a workaroud, Sascha will quickly fix this later he said the minize button (+ -) has a problem, we have an idea on how to fix that thanks to Ankit Jain the lookups that call calendars should not be layered until we solve the issue with them. I think Sascha will be able to solve that when he will come back 2 inputs fields in FTL are shared (see my comment above) and we will need to keep them as is as long as we don't find an answer for them. The only drawback is the button image in Tomahawk. So nothing is really blocking now and we should be ok to commit all soon. Thanks for commiting you patch and updating mine
        Hide
        Jacques Le Roux added a comment -

        This last patch is complete. I'd appreciate reviews before commiting because I did a lot of (trivial) changes by hand and have my eyeballs a little tired. Anyway I should commit in 2 or 3 days...

        Show
        Jacques Le Roux added a comment - This last patch is complete. I'd appreciate reviews before commiting because I did a lot of (trivial) changes by hand and have my eyeballs a little tired. Anyway I should commit in 2 or 3 days...
        Hide
        Bilgin Ibryam added a comment -

        Jacques,

        I applied your patch, browsed a little bit and didn't notice anything wrong.

        Bilgin

        Show
        Bilgin Ibryam added a comment - Jacques, I applied your patch, browsed a little bit and didn't notice anything wrong. Bilgin
        Hide
        Jacques Le Roux added a comment -

        Thanks for the help Bilgin,

        This weekend, I spent a little time on setting layer and topleft as default. I got an issue with the lookup from lookup case (which are working now if you pass the attribute values). I'm still investigating...

        Show
        Jacques Le Roux added a comment - Thanks for the help Bilgin, This weekend, I spent a little time on setting layer and topleft as default. I got an issue with the lookup from lookup case (which are working now if you pass the attribute values). I'm still investigating...
        Hide
        Jacques Le Roux added a comment -

        Commited in trunk at r931854

        Show
        Jacques Le Roux added a comment - Commited in trunk at r931854
        Hide
        Blas Rodriguez Somoza added a comment -

        It seems that there is an error in
        /product/webapp/catalog/category/EditCategoryProducts.ftl
        Line 151
        <@htmlTemplate.lookupField formName="addProductCategoryMemberForm" name="productId" id="productId" lookupFieldFormName="LookupProduct"/>

        AFAIK the last parameter should be fieldFormName instead lookupFieldFormName.

        From Catalog->Browse catalog->Products
        Macro lookupField has no such argument: lookupFieldFormName The problematic instruction: ---------- ==> macro lookupField [on line 22, column 1 in component://common/webcommon/includes/htmlTemplate.ftl] in user-directive htmlTemplate.lookupField [on line 151, column 25 in component://product/webapp/catalog/category/EditCategoryProducts.ftl]

        Show
        Blas Rodriguez Somoza added a comment - It seems that there is an error in /product/webapp/catalog/category/EditCategoryProducts.ftl Line 151 <@htmlTemplate.lookupField formName="addProductCategoryMemberForm" name="productId" id="productId" lookupFieldFormName="LookupProduct"/> AFAIK the last parameter should be fieldFormName instead lookupFieldFormName. From Catalog->Browse catalog->Products Macro lookupField has no such argument: lookupFieldFormName The problematic instruction: ---------- ==> macro lookupField [on line 22, column 1 in component://common/webcommon/includes/htmlTemplate.ftl] in user-directive htmlTemplate.lookupField [on line 151, column 25 in component://product/webapp/catalog/category/EditCategoryProducts.ftl]
        Hide
        Bilgin Ibryam added a comment -

        Thanks Blas, fixed in rev 933108

        Show
        Bilgin Ibryam added a comment - Thanks Blas, fixed in rev 933108
        Hide
        Jacques Le Roux added a comment -

        I also fixed 2 such wrong parameters names at r933145

        Show
        Jacques Le Roux added a comment - I also fixed 2 such wrong parameters names at r933145

          People

          • Assignee:
            Bruno Busco
            Reporter:
            Bruno Busco
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development