CSS-ID/Class settings are ignored - Githubissues

terminal42 / contao-node

Manage content centrally as nodes and reuse them everywhere.

MIT License

29 stars 8 forks source link

CSS-ID/Class settings are ignored #13

Closed saress closed 5 years ago

saress commented 5 years ago

If you add a content-node to an article, it would be great, if you would check if there are some expert-settings available (CSS-ID/Classes) and output them to the frontend.

Right now the ce_nodes.html template only includes:

<?= implode("\n", $this->nodes) ?>

and therefore the CSS-ID/Class settings are ignored. I like it, that nothing else is added to the output, but in this case I would expect an additional div container.

Best regards, Sares

qzminski commented 5 years ago

It's rather a bug that there is a cssID field for content element. It should be removed. @Toflar what do you think?

Toflar commented 5 years ago

We can support both, I think 🤔 If you don't specify anything, we're not wrapping it in another <div> and if you want to specify, we have to wrap it. Wdyt?

fritzmg commented 5 years ago

Imho it should be configurable, whether or not a wrapper should be rendered (regardless of a given CSS ID or class). Similar to the articles of the Contao core.

saress commented 5 years ago

@fritzmg: but what, if you don't have enabled a wrapper and you add some CSS/ID-Class? Where do you render it then?

fritzmg commented 5 years ago

If you want a wrapper, you need to enable the wrapper of course ;)

saress commented 5 years ago

Okay, that makes sense :-)

fritzmg commented 5 years ago

The way I see it the following changes would need to be implemented:

The functions generateSingle, generateMultiple and generateBuffer need a second parameter bool $renderWrapper.
generateBuffer needs to be able to optionally render a wrapper which includes the given CSS-ID/class.
The nodes content element and nodes module need an option whether you want the wrapper to be rendered.
The insert tag needs to have a flag for that.

qzminski commented 5 years ago

@fritzmg I think it doesn't have to be that complicated. See my implementation in d82581e.

fritzmg commented 5 years ago

@qzminski I'd prefer that option in the service - because in one project I was already missing that ;) (but it wasn't strictly necessary, so I made no such suggestion yet).

qzminski commented 5 years ago

It should not be there because a service is not responsible of generating any extra markup. You still have to display the buffer somewhere so it shouldn't be a big deal if you just add some extra HTML around it.

fritzmg commented 5 years ago

Well, in essence I want a function or service that works the same (but better) as Contao's own \Controller::getArticle(…). There you can control as well wether you want to render the wrapper or not.