Skip to content
This repository has been archived by the owner on Jan 1, 2020. It is now read-only.

Provide full access to node rendering API in Twig extension #44

Closed
flack opened this issue Dec 19, 2012 · 8 comments
Closed

Provide full access to node rendering API in Twig extension #44

flack opened this issue Dec 19, 2012 · 8 comments

Comments

@flack
Copy link
Collaborator

flack commented Dec 19, 2012

ATM, the Twig extension can only render a node's attributes and content. It would be good if it could also render the opening and closing tags or the entire node at once. This would allow users to address the problem with duplicate about attributes mentioned here: #29 (comment)

@dbu: I can add the necessary code to CreatephpExtension, but I still don't have a symfony2 testbed (maybe I'll do that over the holidays...), so any testing would have to be done by you or @adou600

@adou600
Copy link
Contributor

adou600 commented Jan 4, 2013

@flack great! I can do the testing if you want. If it comes before the last week of January I can do it without problems, after it is unsure...

I can also help you set up a testbed with Symfony2 and the CMF if you prefer. The easiest solution would be to set up the CMF Website: https://github.com/symfony-cmf/symfony-cmf-website.

The "cmf" branch contains all the changes linked to content creation, as it is on this PR: symfony-cmf/symfony-cmf-website#9. You would just need to undo the changes from: symfony-cmf/symfony-cmf-website@2e4c058 to see the 4 add buttons. Once the website is running, just go to the url http://cmf-website.lo/app_dev.php/login, type username: editor, and password: dummy. Finally go the the page http://cmf-website.lo/app_dev.php/news and you can start playing with the new creation.

The important files to look at would be: https://github.com/symfony-cmf/symfony-cmf-website/blob/cmf/src/Cmf/MainBundle/Resources/views/Cms/news_overview.html.twig and https://github.com/symfony-cmf/symfony-cmf-website/tree/cmf/src/Cmf/MainBundle/Resources/rdf-mappings

Tell me if you have questions or problems with the setup. I will try to make the last changes to symfony-cmf/symfony-cmf-website#9 so that it can be merged soon and make the setup a bit easier.

@dbu
Copy link
Collaborator

dbu commented Jan 9, 2013

we do have tests that render stuff in twig. maybe you can var_dump/echo in them to see what is going on, if you have no symfony testbed running. on the other hand, i think it should not be very difficult to setup symfony cmf sandbox and start playing in it. i can certainly help you if you have questions. (you don't happen to be in switzerland anytime soon, so we could do another hackday?)

@flack
Copy link
Collaborator Author

flack commented Jan 9, 2013

Well, at least until the end of January, I'll be approx. 8000km away from Switzerland, so no dice :-) Maybe at some later point, though. In the meantime, the necessary change is trivial, effectively, we just need to patch the renderStart and renderEnd methods through exactly like renderAttributes and renderContent. But like you mentioned in the other comment, renderStart would of course go against the philosophy of separating metadata and HTML authoring

@dbu
Copy link
Collaborator

dbu commented Jan 9, 2013

i think calling renderStart and renderEnd automatically in the
{% createphp %} command makes more sense. with as i said the option to
supress the output to still have full control over the html authoring
but still let createphp know it is currently inside rendering that node.

@flack
Copy link
Collaborator Author

flack commented Jan 9, 2013

Well, if you call renderStart, but suppress the output, then the about attribute will not get rendered at all, unless I'm missing something. Because then, renderAttributes will look at the isRendering flag, see that it's true and then remove the about attribute before returning the string.

Also, even if it would work as expected, it still would not be bulletproof. e.g. if I have something like this:

{% createphp cmfMainContent as="rdf" %}

<div class="newsoverview" {{ createphp_attributes(rdf) }}>
    xxx
</div>

<ul {{ createphp_attributes( rdf.children ) }}>
    {% for news in cmf_children(cmfMainContent) %}
    {% createphp news %}
         XXX
    {% endcreatephp %}
    {% endfor %}
</ul>

{% endcreatephp %}

Supposing that {% createphp %} doesn't do any output, both createphp_attributes calls would have to render the about tag. However, if we change the markup slightly:

{% createphp cmfMainContent as="rdf" %}
<div class="newsoverview" {{ createphp_attributes(rdf) }}>
    xxx

  <ul {{ createphp_attributes( rdf.children ) }}>
    {% for news in cmf_children(cmfMainContent) %}
    {% createphp news %}
         XXX
    {% endcreatephp %}
    {% endfor %}
  </ul>

</div>

{% endcreatephp %}

then suddenly only the first createphp_attributes call has to render the about attribute. Maybe I'm missing something, but I don't see how the twig extension could know when it is appropriate to render the about attribute or not

@dbu
Copy link
Collaborator

dbu commented Jan 10, 2013

my suggestion would be that

{% createphp cmfMainContent as="rdf" %}
   ...
{% endcreatephp %}

would output the opening tag and attributes and at the end the closing tag. only if you pass an additional option to tell createphp you know what you are doing, it will not output anything, but trigger the information that we are "inside" rendering. the attributes output would then output the "about" information and its the responsibility of the user to not have children outside his tag. he is passing a special attribute, so he is supposed to know what he is doing.

{% createphp cmfMainContent as="rdf" noautotag %}
    <div class="newsoverview" {{ createphp_attributes(rdf) }}>
   ...
{% endcreatephp %}

for the second option, we would need to add a method to be able to notify the entity that it should consider itself being in render mode but not supress the about in its attributes. but supress the about in collections.

@flack
Copy link
Collaborator Author

flack commented Feb 4, 2013

@dbu: OK, now I get it. I left a few comments in the PR, we can continue the discussion there

@flack
Copy link
Collaborator Author

flack commented Feb 15, 2013

#46 has been megred now, so I guess we can close this issue as fixed

@flack flack closed this as completed Feb 15, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants