Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: WW 2.2.2, 2.0.0
    • Fix Version/s: 2.3.16
    • Component/s: Plugin - Tags
    • Labels:
      None
    • Flags:
      Patch

      Description

      If you look at one of the xhtml themes, for example, text.ftl, it is hard coded to use the xhtml controlfooter. This poses a problem for overridding the theme.

      Example xhtml/text.ftl:

      <#include "/${parameters.templateDir}/${parameters.theme}/controlheader.ftl" />
      <#include "/${parameters.templateDir}/simple/text.ftl" />
      <#include "/${parameters.templateDir}/xhtml/controlfooter.ftl" />
      

      Notice the controlfooter does not use ${parameters.theme} but rather is hard coded to xhtml.

        Issue Links

          Activity

          Hide
          Rainer Hermanns added a comment -
          Show
          Rainer Hermanns added a comment - WW issue at OpenSymphony: http://jira.opensymphony.com/browse/WW-1281
          Hide
          Don Brown added a comment -

          Is the fix as simple as s/xhtml/\$

          {parameters.theme}

          / ? If I don't hear otherwise, I'll make the change.

          Show
          Don Brown added a comment - Is the fix as simple as s/xhtml/\$ {parameters.theme} / ? If I don't hear otherwise, I'll make the change.
          Hide
          Alexandru Popescu added a comment -

          I think that indeed that is the fix.

          ./alex

          .w( the_mindstorm )p.

          (http://themindstorms.blogspot.com)

          Show
          Alexandru Popescu added a comment - I think that indeed that is the fix. ./alex – .w( the_mindstorm )p. — ( http://themindstorms.blogspot.com )
          Hide
          Rainer Hermanns added a comment -

          Nope, it is not that trivial...

          I tried it locally, but there is a problem with the parse directive, trying to embed unavailable files within the different themes.
          #parse does use the $

          {parameters.templateDir}

          but does not know what to use, if the requested file is not available.

          If we want to address this problem these might be pssoble solutions:
          o every theme has to include all template files so that the above parameter works as expected.
          If a template is inherited, just use it as a wrapper for a parse call to the correct ('super') theme template file.

          o rewrite or implement an "intelligent" parse macro, that takes the $parameters.templateDir as a default and tries to lookup
          the theme inheritence hierarchy until the named file is found

          What do you think?

          Rainer

          Show
          Rainer Hermanns added a comment - Nope, it is not that trivial... I tried it locally, but there is a problem with the parse directive, trying to embed unavailable files within the different themes. #parse does use the $ {parameters.templateDir} but does not know what to use, if the requested file is not available. If we want to address this problem these might be pssoble solutions: o every theme has to include all template files so that the above parameter works as expected. If a template is inherited, just use it as a wrapper for a parse call to the correct ('super') theme template file. o rewrite or implement an "intelligent" parse macro, that takes the $parameters.templateDir as a default and tries to lookup the theme inheritence hierarchy until the named file is found What do you think? Rainer
          Hide
          Don Brown added a comment -

          I think we should do both. For this ticket, I'll make the change to use the current theme, meaning when making a new theme, you have to copy over all the xhtml theme templates.

          Then, if desired, someone can open a new ticket to implement a theme hierarchy system that makes it easier to create your own theme.

          Show
          Don Brown added a comment - I think we should do both. For this ticket, I'll make the change to use the current theme, meaning when making a new theme, you have to copy over all the xhtml theme templates. Then, if desired, someone can open a new ticket to implement a theme hierarchy system that makes it easier to create your own theme.
          Hide
          Don Brown added a comment -

          Looking into this further, only a properly implemented theme inheritance is going to resolve this. The problem is even our own themes pull from other themes, such that simply replacing the hardcoded xhtml with the dynamic theme name won't work. Therefore, I'm changing this to a new feature request since it does work as advertised.

          Show
          Don Brown added a comment - Looking into this further, only a properly implemented theme inheritance is going to resolve this. The problem is even our own themes pull from other themes, such that simply replacing the hardcoded xhtml with the dynamic theme name won't work. Therefore, I'm changing this to a new feature request since it does work as advertised.
          Hide
          Don Brown added a comment -

          I'll leave this one for folks who use this feature more.

          Show
          Don Brown added a comment - I'll leave this one for folks who use this feature more.
          Hide
          David DeWolf added a comment -

          Implements Hierarchical Theme Loading for Freemarker templates and provides unit tests for the core changes.

          This patch only includes the API changes, not the actual template changes needed to leverage this in core templates. The ftl changes may be very painful as there are currently many hacks to get around the fact that this isn't currently implemented.

          Show
          David DeWolf added a comment - Implements Hierarchical Theme Loading for Freemarker templates and provides unit tests for the core changes. This patch only includes the API changes, not the actual template changes needed to leverage this in core templates. The ftl changes may be very painful as there are currently many hacks to get around the fact that this isn't currently implemented.
          Hide
          Don Brown added a comment -

          Moving to future as this is a pretty big change, with a lot of work still to do.

          Show
          Don Brown added a comment - Moving to future as this is a pretty big change, with a lot of work still to do.
          Hide
          tibi added a comment -

          maybe a lot of files to fix but the fix has a minor impact and easy to fix.

          Show
          tibi added a comment - maybe a lot of files to fix but the fix has a minor impact and easy to fix.
          Hide
          Lukasz Lenart added a comment -

          tibi could you update the patch to the latest version ? I was trying to apply this patch but many tests have failed

          Show
          Lukasz Lenart added a comment - tibi could you update the patch to the latest version ? I was trying to apply this patch but many tests have failed
          Hide
          Lukasz Lenart added a comment -

          Theme inheritance improved with WW-4145

          Show
          Lukasz Lenart added a comment - Theme inheritance improved with WW-4145

            People

            • Assignee:
              Lukasz Lenart
              Reporter:
              Nick Hill
            • Votes:
              4 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development