Project

General

Profile

Actions

Feature #71484

closed

Output of layout field in fluid_styled_content templates

Added by Wolfgang Wagner over 8 years ago. Updated over 6 years ago.

Status:
Closed
Priority:
Should have
Category:
Fluid Styled Content
Target version:
Start date:
2015-11-11
Due date:
% Done:

100%

Estimated time:
PHP Version:
Tags:
Complexity:
Sprint Focus:

Description

It would be nice, if the layout field in content elements would be included in the templates of FSC.
At the moment it is necessary to manually adjust the templates.


Files

layout_modified.zip (666 KB) layout_modified.zip Matthias Wehrlein, 2016-05-18 10:59

Related issues 1 (0 open1 closed)

Has duplicate TYPO3 Core - Bug #71490: fluid-styled-content does not render Layout field into HTML outputClosedBenni Mack2015-11-11

Actions
Actions #1

Updated by Mathias Schreiber over 8 years ago

  • Status changed from New to Needs Feedback
  • Assignee set to Mathias Schreiber

But it's only necessary, if you use that field, right?

Actions #2

Updated by Wolfgang Wagner over 8 years ago

Sure, but the possibility to use that is given per default. So, if a user selects another layout, there is no effect in the FE. And this is not a good UX.

Actions #3

Updated by Mathias Schreiber over 8 years ago

What would be the expected change in the Frontend?
Add a class attribute?

Actions #4

Updated by Wolfgang Wagner over 8 years ago

Yes, maybe something like

class="layout-{data.layout}"

Actions #5

Updated by Mathias Schreiber over 8 years ago

How does this change the frontend then (as expected by the user)? :)

Actions #6

Updated by Wolfgang Wagner over 8 years ago

Of course you have to adapt your css, but this was necessary too in csc.

But it's easier to write a few lines of css than to copy und adjust the fsc templates ;)

Actions #7

Updated by Jörg Wagner over 8 years ago

The changes to implement this would be minimal:

Change all occurences of...

<div id="c{data.uid}">

...within the FSC template files to...
<div id="c{data.uid}" class="layout-{data.layout}">

This would also render out the default setting "Normal" as class="layout-0".

If "layout-0" is unwanted (I would not see why), the 0-case can be eliminated with an additional condition:

<div id="c{data.uid}"<f:if condition="{data.layout}"> class="layout-{data.layout}"</f:if>>

PS:
All this being said I am waiting for the day where all TYPO3 CEs will support a multiselect field where each selection is rendered out as a separate class name. ;D

Actions #8

Updated by Mathias Schreiber over 8 years ago

The entire idea of FSC was to satisfy the demand for CEs without unnecessary HTML markup.
So there are a couple of points speaking against adding this:
  • we do not want bloated HTML markup
  • it is easy to implement (just override the three files in an extension and you're done)
  • classnames like layout-0, layout-1, layout-2 and section-frame-5050 are absolutely pointless in your CSS. Nobody will know what they do.

I think I can come up with a couple more if I just take my time.
So I will keep this ticket updated.

Actions #9

Updated by Wolfgang Wagner over 8 years ago

I know how to fix that, but I think, if there is the option to select a layout in the backend, then should it be usable without the need to change the default templates.

Actions #10

Updated by Mathias Schreiber over 8 years ago

Ok, so following that principle we add:
  • header_position
  • section_frame
  • spaceBefore (my favourite)
  • spaceAfter (again, brilliant)
  • categories (how do you want them rendered? inline or as an unordered list?)

Do you see where this is going? :D

Actions #11

Updated by Jörg Wagner over 8 years ago

@Mathias:
header_position, section_frame, spaceBefore, spaceAfter ... are all GONE from the backend AND from the database.
But Layout is still in the DB and in the backend when editing CEs
There has obviously been a decision to keep this single attribute and this issue is about the discrepancy to show it in the backend but not to render it out at all into HTML.

Actions #12

Updated by Mathias Schreiber over 8 years ago

Question remains... how do you want the categories to be rendered?

Actions #13

Updated by Jörg Wagner over 8 years ago

I do not.
Mathias, you are making up arguments.

Actions #14

Updated by Mathias Schreiber over 8 years ago

See, here's the thing:
Having a field in the Backend does not necessarily mean it has an direct effect on the output of the content elements in the Frontend.
Example: The categories field.
I think we can agree that this field is indeed there.
So all I do is take your argument (field is there, should to things in the Frontend) and apply it to all fields in an Content Element (category selection is there, why do I not see it in the frontend?)

I do not see how this is a "made up" argument, please explain it to me.

Actions #15

Updated by Jörg Wagner over 8 years ago

Ok, you are right. Here is my argument:
  • Layout has been an attribute that was rendered out (as layout-1, -2, ...) for years, AFAIK at least since TYPO3 3.8. If we keep the field, why should we drop the rendering.
  • Rendering it out would allow for an easier migration of pre-7 systems.
  • Using my second replacement proposal it will only be rendered out when it is actually used = set to something else than "Normal". The HTML would not be cluttered at all.
  • I agree that layout-1 is not speaking and in pre-7 we added our own class names. But I still prefer to have something about nothing.
  • Categories are a completely different concept. They can be nested. They have a general use case of being filters. It is good that they are there but their target is surely not to be added as class names – at least not in any default setup. They are also very new to TYPO3 CEs and there is no need at all for any consistency with older systems.
Actions #16

Updated by Wolfgang Wagner over 8 years ago

Wether the class are called layout-0, layout-1 or something else doesn't matter, I think. The old csc-frame-frame1 was not better ;)

I agree to your arguments, Jörg!

(Btw: I regret the loss of the section_frames. In combination with layout, great flexibility in giving certain classes to CE's was possible. But this is another story.)

Actions #17

Updated by Alexander Opitz about 8 years ago

  • Status changed from Needs Feedback to New
Actions #18

Updated by Riccardo De Contardi about 8 years ago

I'll try to say my 2 cents about this topic.

If there would be inside the CSS shipped with TYPO3 some default style definitions for layout-0 , layout-1 and so forth, then yes, these classes should be rendered too by default on the FSC elements. As there are no css rules associated to these classes (AFAIK) , they have been removed, and the integrators are free to add them again and use {data.layout} as they prefer (maybe I want to use the BEM notation and write something like contentcolumn__element--layout-0, someone else would like a different class et cetera).

In other words: if these classes means something in your css, then it is you that have to add them to the HTML template, otherwise it is true that they are "cluttering" the html

I regret the loss of the section_frames. In combination with layout, great flexibility in giving certain classes to CE's was possible.

Yes, I think I'll miss them too :)

I am waiting for the day where all TYPO3 CEs will support a multiselect field where each selection is rendered out as a separate class name.

I totally agree :)

that of course is my opinion, I hope I've made myself clear. Cheers.

Actions #19

Updated by Riccardo De Contardi about 8 years ago

  • Category set to Fluid Styled Content
Actions #20

Updated by Urs Braem about 8 years ago

I'm more of a header_layout guy :-) – hopefully not anymore...

And I think it would be the wrong direction to start bloating markup again. I used to have about 20 lines of TS of which I didn't have a clue what it was doing, labeled "remove some css_styled_content classes and unnecessary stuff".
As long as it's possible to access the data in a custom template that's easily accessed (as it is with fsc), it would be a very bad idea to start adding classes.

Actions #21

Updated by Jörg Wagner about 8 years ago

As pointed out earlier, the mark-up is not bloated in any way: As long as the layout dropdown is not used, no additional markup is generated.
If the layout dropdown is changed to something else than its default value, than the markup MUST reflect this, if you want to have any effect.

I understand that you can argue, that the layout field is only provided for convenience and rendering must be addressed by the integrator if needed. But then you introduce a breaking change with any pre-7 templating and upgrades from 6.2 or earlier become unnecessarily painful.

Now, breaking changes have happend and will happen again. They are in fact often needed.
But I argue that IF we introduce a breaking change concerning the layout field, then we should do it right:
The layout field should finally become a multiselect dropdown when FSC is chosen as the rendering method.
A CSS class multiselect is overdue in all of TYPO3 for a long time and this would be the chance to introduce it.

Actions #22

Updated by Wolfgang Wagner about 8 years ago

Jörg Wagner wrote:

The layout field should finally become a multiselect dropdown when FSC is chosen as the rendering method.
A CSS class multiselect is overdue in all of TYPO3 for a long time and this would be the chance to introduce it.

That would be cool!

Actions #23

Updated by Matthias Wehrlein almost 8 years ago

I'm impressed by your stubbornness, Mathias!

Having a field in the Backend does not necessarily mean it has an direct effect on the output of the content elements in the Frontend.

You can't really compare a field labelled "Layout" with something like categories.

I don't expect categories to have an immediate effect on frontend output, but c'mon, a field called "Layout" can't be more suggestive. You are making up arguments with this.

Right now, the way I see it, "Layout" is there and suggest it does something, but it does not. And it's not like just changing three files; to do it properly, you'd have to touch every content elemente template.

we do not want bloated HTML markup

Adding a class is not exactly bloat, is it? It provides easily implemented flexibility.

it is easy to implement (just override the three files in an extension and you're done)

See above. To do it properly, this is more than just three files.

classnames like layout-0, layout-1, layout-2 and section-frame-5050 are absolutely pointless in your CSS. Nobody will know what they do.

But they provide an easy starting point. Your argument is just inappropriate and overly sophisticated semantic wish-wash. Form follows function. And it's not like you can't change those things to something more meaningful via TCA.

Just my three cents.

Actions #24

Updated by Mathias Schreiber almost 8 years ago

Matthias Wehrlein wrote:

I'm impressed by your stubbornness, Mathias!

I don't really see why I'm stubborn while others also raise the same concerns as me, but ok - to each his own.
We have different opinions in this discussion (not only brought up by me I might add) and being stubborn would be to either accept the change and add the "dead" markup or close the ticket.
Neither of this happened, so lets refrain from personally attacking people, deal?

You can't really compare a field labelled "Layout" with something like categories.

I don't expect categories to have an immediate effect on frontend output, but c'mon, a field called "Layout" can't be more suggestive. You are making up arguments with this.

I'd prefer it if you would not tell me what I do and what I do not do.
Stating someone "makes up arguments" works in both ways and we should avoid this type of discussion - simply because it won't get anyone anywhere.

On the matter:
I did not close the ticket, mostly because I think the idea of having a list of selectable classes to be rendered space-separated is a pretty good idea and would be a solution far superior to just adding "some" default class attribute.
This way you would have more flexibility in regards to adding classes and furthermore we would keep up with the trend of more modular CSS in general.

So as soon as someone with basic TS or PHP knowledge picks up the task I am happy with merging it into the core.

Actions #25

Updated by Matthias Wehrlein almost 8 years ago

Hey, absolutely no offense intended, and least of all I wanted it to come across as personally attacking you. Honestly sorry for that.

Anyways, as for the regression at hand, I actually already did implement what Jörg suggested in comment #7, as I also think it's the most flexible solution to this:

All this being said I am waiting for the day where all TYPO3 CEs will support a multiselect field where each selection is rendered out as a separate class name. ;D

I have this implemented in an extension that overrides the TCA for the layout field providing some more meaningful values and also overrides templates from fluid_styled_content to actually make use of those classes. It works quite well. Also, I took the time to look up where to modify the remaining content elements that ship with TYPO3 (login, mailform, list).

Probably I'd be able to provide a somewhat usable patch.

There's just one catch: This way the layout field is stored as a comma-separated list and currently I'm using vhs to explode this. I thought about preprocessing this somewhere else (FluidTemplateContentObject or maybe ContentObjectRenderer, no idea) but that seemed like a bad idea.

So, imho, that would require a fluid-provided explode view helper, and well … that's just way over my head. I'm not so much into unit-testing and all that fancy stuff.

Actions #26

Updated by Riccardo De Contardi almost 8 years ago

@Matthias Wehrlein:

using a dataProcessor? (https://docs.typo3.org/typo3cms/extensions/fluid_styled_content/7.6/AddingYourOwnContentElements/Index.html#data-processor)

I would be very interested to look at your extension, can you share it?

Actions #27

Updated by Matthias Wehrlein almost 8 years ago

Hey Riccardo, sure.

I attached a ZIP archive that should contain the essential things. It has been part of an extension that provides layouts via flux/fluidpages/fluidcontent/vhs and I stripped it of most things. The dependency on flux/fluidpages/fluidcontent shouldn't be necessary anymore so you might wanna remove those from ext_emconf.php, in addition to the flux calls in ext_localconf.php.

The important bits are (in no particular order):

  • Configuration/TCA/Overrides/tt_content.php modifies the TCA to make tt_content.layout a multiselect field and add the desired values (didn't use translation mechanism for those, been too lazy).
  • Configuration/TypoScript/setup.txt add paths to override fluid_styled_content layouts/templates/partials.
  • ext_tables.sql changes tt_content.layout from `int` to `varchar(255)` so that it can actually store the new values.
  • Resources/Private/Extensions/fluid_styled_content/Layouts and Resources/Private/Extensions/fluid_styled_content/Layouts hold the necessary template overrides that actually render those newly added layout options (what all the fuss in this ticket is about). Templates/Textmedia.html is special in that it foregos using a layout file for whatever reason. Anyways, in those file you can see the use of {v:format.replace}, where I "split" the CSS classes.

If you have question, don't hesitate to get back at me.

And thanks for your link, that looks very promising! Though I still think that isn't the best solution for a core provided solution. Other things might modify tt_content.layout like I did and then you might not want to have these things preprocessed. But I will definitely investigate that for my own use case.

Actions #28

Updated by Riccardo De Contardi almost 8 years ago

@Matthias Wehrlein thank you for your time and your answer!

well, dataProcessors are part of core and as far as I know are currently used for example in the "bullet list" CE to "split" the content of the bodytext element and render it into a list :) Maybe there is already a processor that splits a list of comma separated values

Have a nice day and good work!

Actions #29

Updated by Riccardo De Contardi almost 8 years ago

@Matthias Wehrlein, @everyone

just in case someone needs: if you want to use the dataProcessor, simply put in TS Setup:

lib.fluidContent{
  dataProcessing.99 = TYPO3\CMS\Frontend\DataProcessing\SplitProcessor
  dataProcessing.99{
     if.isTrue.field = layout
     delimiter =,
     fieldName = layout     
     as = splitlayout  
  }
}

and then in the fluid template:

  <f:for each="{splitlayout}" as="layout">{layout} </f:for>

Actions #30

Updated by Matthias Wehrlein almost 8 years ago

Dear sir, that is awesome! Thanks for providing that example; was literally giving it a go right, then saw the forge notification.

Great stuff, really makes vhs redundant in this case. Thanks again!

Actions #31

Updated by Christoph Werner over 7 years ago

I´ve just migrated a system from css-styled-content to fluid-styled-content... well, that point and some others are really annoying.

Having a field an not effect on the output is a bad thing and a point why newbies will dislike TYPO3.. keeping the code clean is one good thing, but logic (use a field, get some output change) is more important.

The multiselect dropdown when FSC is activ is a great idea and would be perfekt to substitute the loss the "section and frames".

Best
Chris

Actions #32

Updated by Riccardo De Contardi over 7 years ago

If someone is interested (but this is not the place for a full discussion, feel free to contact me on Slack) , in my experience I found useful to add to the outermost <div> of Content Elements also some classes that represent the CType and the list_type (for plugins), something like that (using the splitlayout I mentioned on my previous comment:

<div id="c{data.uid}" class="type-{data.CType} {f:if(condition: data.list_type, then: 'subtype-{data.list_type}')} <f:for each="{splitlayout}" as="layout">{layout} </f:for>">
Actions #33

Updated by Jacco van der Post over 7 years ago

In my opinion id="c{data.uid}" shouldnt even be in the fluid templates. It really annoys me that I have to overwrite for every site I make:

  1. Textmedia.html
  2. ContentFooter.html
  3. HeaderContentFooter.html
  4. HeaderFooter.html

Just to add core functionality 'layout' there.

I would say make a typoscript setting of it, so anything can be added there easily.

Actions #34

Updated by Jörg Wagner over 7 years ago

Jacco van der Post wrote:

In my opinion id="c{data.uid}" shouldnt even be in the fluid templates. ...

The cUID id attribute is necessary for in-page links (AKA fragments, hash links) of the form: index.php?id=123#c456
You can create such links in RTE with the normal link tool by selecting not only a target page but also a content element within that page.

Actions #35

Updated by Gerrit Code Review about 7 years ago

  • Status changed from New to Under Review

Patch set 66 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #36

Updated by Gerrit Code Review about 7 years ago

Patch set 67 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #37

Updated by Gerrit Code Review about 7 years ago

Patch set 68 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #38

Updated by Gerrit Code Review about 7 years ago

Patch set 69 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #39

Updated by Gerrit Code Review about 7 years ago

Patch set 70 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #40

Updated by Gerrit Code Review about 7 years ago

Patch set 71 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #41

Updated by Gerrit Code Review about 7 years ago

Patch set 72 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #42

Updated by Gerrit Code Review about 7 years ago

Patch set 73 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #43

Updated by Gerrit Code Review about 7 years ago

Patch set 74 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #44

Updated by Gerrit Code Review about 7 years ago

Patch set 75 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #45

Updated by Gerrit Code Review about 7 years ago

Patch set 76 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #46

Updated by Gerrit Code Review about 7 years ago

Patch set 77 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #47

Updated by Gerrit Code Review about 7 years ago

Patch set 78 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #48

Updated by Gerrit Code Review about 7 years ago

Patch set 79 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #49

Updated by Gerrit Code Review about 7 years ago

Patch set 80 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #50

Updated by Gerrit Code Review about 7 years ago

Patch set 81 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #51

Updated by Gerrit Code Review about 7 years ago

Patch set 82 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #52

Updated by Gerrit Code Review about 7 years ago

Patch set 85 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #53

Updated by Gerrit Code Review about 7 years ago

Patch set 86 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/51065

Actions #54

Updated by Anonymous about 7 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions #55

Updated by Riccardo De Contardi over 6 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF