sitegeist / fluid-components

Encapsulated frontend components with Fluid's ViewHelper syntax for TYPO3
https://fluidcomponents.sitegeist.de/
GNU General Public License v2.0
54 stars 21 forks source link

Trim whitespaces in slot before counting content #135

Closed sascha-egerer closed 1 year ago

sascha-egerer commented 1 year ago

The slot model implements the Countable interface that is used when evaluating the variable in an if condition. Before counting the content by calling strlen it should be trimmed so whitespaces, that are common in HTML, are ignored.

Example

...
<fc:content name="foo">

</fc:content>
...

Checking the Slot "foo" in a condition should be false as the slot does not contain real content.

<f:if condition="{foo}">this should not be shown</f:if>

s2b commented 1 year ago

It could also make sense to trim the output, similar to the fc:component ViewHelper. What do you think?

sascha-egerer commented 1 year ago

It could also make sense to trim the output, similar to the fc:component ViewHelper. What do you think?

I had this before but not sure if someone does use it where he needs whitespaces in the output like a text field. So i just did it for the count part. For me it would be finde to also trim the output itself but not sure if it would break something...

sascha-egerer commented 1 year ago

One could also argument this change should not be merged as the behavior is different to "normal variables". So if you have a normal variabel the behavior is the same.

<f:variable name="foo">

</f:variable>

<if condition="{foo}">foo is true</f:if>
<if condition="{foo -> f:spaceless}">this will not be rendered as foo is now false</f:if>

So the question is how a slot should behave...

s2b commented 1 year ago

Yes, I think it should be consistent between the <f:if> and a simple variable output... I'll think about it. If you have any further input, let me know.

s2b commented 1 year ago

I discussed this with @Atomschinken internally: We prefer to stay consistent here and to not trim the input of Slots by default. However, it could make sense to offer a setting (feature flag?) for this, which would make this non-breaking. This also means that this is not time-critical and can be implemented after the breaking version that adds v12 compatibility.

sascha-egerer commented 1 year ago

I think a feature flag is a very bad idea as components will behaves differently depending on this setting. So I'm fine with closing this issue even if i do not like the behavior as it can be very confusing if a slot does not contain content beside whitespaces but is handeled like it does contain content.

s2b commented 1 year ago

I see your point. Just to give you some context: There could be cases in which the whitespace is relevant, for example:

<my:atom.textarea>
Text in the second line of the textarea</my:atom.textarea>

Maybe this could also be solved together with the named slots feature (#129) as an argument:

<fc:content spaceless="1">
  my content with whitespace
</fc:content>
sascha-egerer commented 1 year ago

Yes, i know.. i already wrote a comment with exactly this case but looks like i forgot to submit it... So as said I'm fine with closing it so i'll close it now.

You're case won't work at least not with the current implementation of named slots as you can't define for the "content" variable. And if something like this is required one could also just add the spaceless viewhelper to the tag (but then it must be without whitespaces around it what could be hard again if HTML linters / autoformaters are used.)

Or maybe just turn it around and make it keepSpaces but this should then false by default which would be a breaking change again. So... I'll close this one and maybe someone comes up with a good solution at some time.