Renderer options (close #642). #795

Merged
merged 1 commit into from Jan 24, 2013

Projects

None yet

3 participants

@jails
Union of RAD member

No description provided.

@jails jails commented on an outdated diff Jan 23, 2013
tests/mocks/template/MockRenderer.php
@@ -11,6 +11,11 @@
class MockRenderer extends \lithium\template\view\Renderer {
public function render($template, $data = array(), array $options = array()) {
+ $this->_options = $options;
@jails
jails Jan 23, 2013

I'm not sure this is allowed ...

@jails
Union of RAD member

Finally the "mocking" way was to "invasive" here. Just pushed a better version.

@nateabele nateabele commented on the diff Jan 24, 2013
template/view/Renderer.php
@@ -487,10 +495,7 @@ public function set(array $data = array()) {
* @return string Returns a the rendered template content as a string.
*/
protected function _render($type, $template, array $data = array(), array $options = array()) {
- if ($this->_request) {
- $library = $this->_request->library;
@nateabele
nateabele Jan 24, 2013

Is the removal of this fixed in some way? Otherwise, you can't correctly render elements in the same library as the template.

@jails
jails Jan 24, 2013

Since the context is saved in File with $this->_options = $options;, $this->_options should already contain the library param passed from the controller. Imo this "forced overriding" can be safely removed.

@tefra
tefra Feb 7, 2013

There is a catch though if you render elements from different libraries, the options are completely overwritten

This example used to work. The sidebar belongs to library B, pager to the library A and the snippet is from a template from library B

<?php echo $this->_render('element', 'pager', array(), array('library' => 'A')); ?>
<?php echo $this->_render('element', 'sidebar'); ?>

If you reverse the order it still works, but in cases where you render multiple elements from multiple libraries you can't simply depend on the order they are called. You have to define the library for all the elements to be safe. I don't know if this is a bug or simply a BC break but i think the original library option should be preserved.

@jails
jails Feb 7, 2013

Well adding the original library using $this->_request->library; is not a good option imo. For example, if you do some cascading calls using ::_render in some library's templates, you'll need to set array('library' => 'A') everywhere. Maybe #817 is a better option.

@nateabele nateabele commented on an outdated diff Jan 24, 2013
template/view/Renderer.php
@@ -132,6 +132,14 @@
*/
protected $_vars = array();
+ /*
+ *Available options accepted by `template\View::render()`, used when rendering.
@nateabele
nateabele Jan 24, 2013

Looks like you missed a space after *.

@jails
Union of RAD member

Just pushed an additionnal test which explicitly check $this->_options['library'] in a view.

@nateabele nateabele merged commit d1d8f74 into UnionOfRAD:dev Jan 24, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment