Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
jonaseberle opened this issue Feb 26, 2019 · 7 comments

Comments

@jonaseberle
Copy link
Contributor

jonaseberle commented Feb 26, 2019

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
Copy link
Contributor

<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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

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:

  • What if you want the content to be ignored instead of the tag (inviting even more complex use cases)?
  • What if you wanted one or more attributes to be ignored?
  • Still won't work with anything except a tag-based ViewHelper.

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
Copy link
Contributor Author

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

  • Either the HTML structure gets raped or HTML blocks get duplicated inside a f:if-then-else.

@NamelessCoder
Copy link
Contributor

NamelessCoder commented Feb 27, 2019

@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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants