-
-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add note about using RendererInterface::render() #39
base: 2.11.x
Are you sure you want to change the base?
Conversation
Signed-off-by: Aleksi Peebles <aleksi@iki.fi>
55790f6
to
1cedb5a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this contribution!
Only small changes are necessary so that the information can be included in the documentation.
Signed-off-by: Aleksi Peebles <aleksi@iki.fi>
> argument. You might prefer to do this if your model is an array, or an | ||
> object that implements `ArrayAccess`. See the documentation for | ||
> [`PhpRenderer`](../php-renderer.md#render) for more information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might prefer to do this if your model is an array, or an object that implements
ArrayAccess
.
The result is the same with the Partial helper and render
method for this.
But the Partial helper supports some more:
laminas-view/src/Helper/Partial.php
Lines 52 to 58 in 2e961a1
if (null !== ($objectKey = $this->getObjectKey())) { | |
$values = [$objectKey => $values]; | |
} elseif (method_exists($values, 'toArray')) { | |
$values = $values->toArray(); | |
} elseif (! $values instanceof Traversable) { | |
$values = get_object_vars($values); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally put the note in the very beginning as I considered it to be quite important for deciding whether to use Partial
or not.
It seems that historically, when render()
did not support a $values
argument, Partial
was much more useful, but what is left now is just this additional support for handling data. One might even consider the current introduction ("is used to render a specified template within its own variable scope") to be misleading?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm referring particularly to this commit, which seems to have changed the nature of Partial
: e0825a0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not put so much emphasis on original or old behavior. The user should receive instructions on how to use the helper. That the render
method is used is an important hint, but it is also important that the helper offers more. (See "What is a model?")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, currently the documentation seems to put too much emphasis on the old variable scope feature that has since been added to RendererInterface
and is now only proxied by Partial
.
I would even consider whether the model handling functionality that is left is valuable enough to keep Partial
instead of deprecating it? The objectKey
feature for example seems to be equivalent to a simple render($nameOrModel, [$key => $object])
call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking into this more I see that Partial
and its model are used by other parts of the framework, so I am guessing that there are good reasons for keeping the additional model support. And maybe in any case for keeping Partial
as the recommended way to always render partial templates, since specific functionality can then be easily added later?
As a developer using the framework for the first time, interested in rendering partial templates in their own variable scope, I have however been confused about:
- the difference between
RendererInterface::render()
andPartial
, and - whether I should still use
Partial
if it only seems to add overhead in my specific use case, where I will not be needing the extended model support.
I am still unsure about the right answers to these questions, and I would love to find the answers in the documentation. This is what I was attempting to do with the PR. If there are additional changes that are needed I am happy to update the PR accordingly, but for the above reasons I would need some more guidance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dislike the entire page and the missing code samples for the different model types.
I will rewrite the page:
- explain the internal functioning (
render
method) - add code samples for all types of models
- extract the PartialLoop helper to a separate page
- add relevant links
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, thank you for bringing up this topic.
Btw. please have a look at the projects, there will you find "Documentation: cookbook recipes".
More ideas are needed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, will have a look!
Description
Adding a note about the possibility to use
RendererInterface::render()
instead.