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

CSS-ID/Class settings are ignored #13

Closed
saress opened this issue Jul 26, 2019 · 11 comments
Closed

CSS-ID/Class settings are ignored #13

saress opened this issue Jul 26, 2019 · 11 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@saress
Copy link

saress commented Jul 26, 2019

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

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

@qzminski qzminski added the question Further information is requested label Jul 26, 2019
@Toflar
Copy link
Member

Toflar commented Jul 26, 2019

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

fritzmg commented Jul 26, 2019

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.

@Toflar Toflar added enhancement New feature or request and removed question Further information is requested labels Jul 26, 2019
@saress
Copy link
Author

saress commented Jul 26, 2019

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

fritzmg commented Jul 26, 2019

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

@saress
Copy link
Author

saress commented Jul 26, 2019

Okay, that makes sense :-)

@fritzmg
Copy link
Sponsor Contributor

fritzmg commented Jul 26, 2019

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

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

@Toflar Toflar added this to the 1.1.0 milestone Jul 26, 2019
@fritzmg
Copy link
Sponsor Contributor

fritzmg commented Jul 26, 2019

@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
Copy link
Member

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

fritzmg commented Jul 26, 2019

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.

@Toflar Toflar mentioned this issue Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants