TYPO3 / Fluid

Fluid template rendering engine - Standalone version
GNU Lesser General Public License v3.0
152 stars 93 forks source link

Feature request: Allow tags to be excluded from rendering / Global Fluid-parameters #430

Open jonaseberle opened 5 years ago

jonaseberle commented 5 years ago

I often feel the need to only render a tag based on conditions, but render its contents anyways.

A simple example would be content that might be needed to be wrapped in a link or not.

To recreate this logic in Fluid, currently the best way seems:

<f:render section="PageLinkWrapMaybe" contentAs="content" arguments="{condition: myCondition}>
  $HTMLToBeWrappedMaybe
</f:render>

<f:section name="PageLinkWrapMaybe">
  <f:if condition={myCondition}>
    <f:then>
      <f:link.page pageUid=...>
        {content}
      </f:link>
    </f:then>
    <f:else>
        {content}
    </f:else>
  </f:if>
</f:section>

Nota bene: Workarounds like

{f:if(condition: myCondition, then: '<a href="{f:uri.page(pageUid: ...)}">', else: '')}
      $HTMLToBeWrappedMaybe
{f:if(condition: myCondition, then: '</a>', else: '')}

are making things complex to read and write and are not sane IMHO. But even given that this is incredibly ugly it is actually used thus proving my point that this use case is very common.

Wouldn't it be a lot easier if I could do

    <f:link pageUid=...
       f:renderTagIf="{myCondition}"
    >
      $HTMLToBeWrappedMaybe
    </f:link>

?

I envision this for any tag (not just Fluid-tags), e.g.

<div class="wrapper" f:renderTagIf="{myCondition}">

So something global, not just an additional ViewHelper-parameter to have it really flexible.

Examples for templating languages where I have seen and learnt to love similar are ZPT (tal:condition https://zope.readthedocs.io/en/latest/zope2book/ZPT.html#conditional-elements and tal:omit-tag) and JSF (the rendered= parameter).

What do you think? Any ideas?

mbrodala commented 5 years ago
<f:if condition={myCondition}>
  <f:then>
    <f:link.page pageUid=...>
      $HTMLToBeWrappedMaybe
    </f:link>
  </f:then>
  <f:else>
      $HTMLToBeWrappedMaybe
  </f:else>
</f:if>

FYI, this could be rewritten using a partial/section:

<f:render section="MyLink" contentAs="label" arguments="{condition: condition}>
  $HTMLToBeWrappedMaybe
</f:render>

<f:section name="MyLink">
<f:if condition={condition}>
  <f:then>
    <f:link.page pageUid=...>
      {content}
    </f:link>
  </f:then>
  <f:else>
      {content}
  </f:else>
</f:if>
</f:section>

Still rather verbose but reduces code duplication.

jonaseberle commented 5 years ago

That's a good idea to reduce duplication. I still don't think it's easy to read but that's for sure the best way right now. I'll update my initial post.

jonaseberle commented 5 years ago

Renamed the attribute from f:wrapIf to f:renderTagIf, too.

f:if might imply that the content is not rendered, f:tagIf might be too compact.

NamelessCoder commented 5 years ago

Hi @jonaseberle - I understand the use case, but <div class="wrapper" f:renderTagIf="{myCondition}"> will never be supported simply because the XHTML tag you render, is not Fluid.

While this might be possible to implement as a shared feature for tag-based ViewHelpers, even that comes with an array of problems:

I would argue that it is not the default use case to want this capability, but more or less only makes sense in context of links (that show text without tag). It is also excessively easy to implement in a tag-based ViewHelper on a case-to-case basis (by simply not building the tag but returning the output of render closure instead). So while it might be nice to have when the use case does arise, I really don't think it makes enough sense in a broader context to enable this on every tag-based ViewHelper.

Did you consider:

{content -> f:link.page(pageUid: 123) -> f:if(condition: myCondition, else: content)}
jonaseberle commented 5 years ago

Thank you for the code snippet. If you have the content as variable, that might help.

I do not agree with your judgement, though.

1) Fluid should not only focus on the "standard case".

2) There are a lot more examples. A quick sweep through fluid_styled_content shows a lot of use cases:

https://github.com/TYPO3-CMS/fluid_styled_content/blob/master/Resources/Private/Partials/Media/Type/Image.html

https://github.com/TYPO3-CMS/fluid_styled_content/blob/master/Resources/Private/Partials/Media/Gallery.html

https://github.com/TYPO3-CMS/fluid_styled_content/blob/master/Resources/Private/Templates/Textpic.html

https://github.com/TYPO3-CMS/fluid_styled_content/blob/master/Resources/Private/Templates/Textmedia.html

https://github.com/TYPO3-CMS/fluid_styled_content/blob/master/Resources/Private/Templates/Table.html

NamelessCoder commented 5 years ago

@jonaseberle Thanks for the examples. For clarity I will go through each one of them, hopefully that illustrates why I make the judgements I make:

https://github.com/TYPO3-CMS/fluid_styled_content/blob/master/Resources/Private/Partials/Media/Type/Image.html

This structure can be cleaned up very significantly by using variable assignments to store the image/tag and conditionally wrapping it using the method I suggested. Additionally, @mbrodala's suggestion about sections to do wrapping also applies (combine that with my code piece for an even more compact structure).

https://github.com/TYPO3-CMS/fluid_styled_content/blob/master/Resources/Private/Partials/Media/Gallery.html

This is not the right way to write a Fluid template and it only works because there is no Fluid code inside those conditions before/after the loop. This should absolutely without question be refactored to conditional wrapping using a section or partial.

https://github.com/TYPO3-CMS/fluid_styled_content/blob/master/Resources/Private/Templates/Textpic.html

This template can be sanitised and compacted in several ways:

  1. Variable assignments can be used to remove duplicated calls to ViewHelpers.
  2. Conditions can be compacted using the if argument on f:else
  3. The conditions themselves can be written into one condition

https://github.com/TYPO3-CMS/fluid_styled_content/blob/master/Resources/Private/Templates/Textmedia.html

Identical to "textpic".

https://github.com/TYPO3-CMS/fluid_styled_content/blob/master/Resources/Private/Templates/Table.html

I agree this template is pretty bad but every suggestion given above actually applies in this template as well.

I do not disagree with you that these templates aren't ideal, but you should be aware that they were written very long ago and don't use the appropriate Fluid features that were created exactly to combat the problems you described. The links aren't so much a complaint against Fluid - rather, a complaint against fluid_styled_content for simply not updating templates as new features became available. I am always available to provide best-usage examples, should someone wish to improve these!

Keeping this new information in mind, could you specify what you disagree with about my judgement? Perhaps we're not completely understanding each other about the "standard case" way of putting it. What I mean is simply that it's not in my opinion reasonable to build a feature into each and every TagBasedViewHelper and by doing so, creating breaking incompatibilities in render methods, to solve a use case that's relevant for perhaps 1% of templates - if that much.

jonaseberle commented 5 years ago

Ok, lets make pull requests to fluid_styled_content and see where we go from there. So to at least have good examples in any default TYPO3 installation ;)

I'll see how I find time.