Updates in Helper\Navigation\Breadcrumbs.php and Menu.php

[Zyqsempai]

Added possibility to set parameters(variables) for partial to populate in the view.
In the current version there is no possibility to transmit variables into partial view , you can only set partial and module like this:

$this->navigation('navigation')
    ->menu()
    ->setPartial('partial/yourPartial.phtml','yourModule');

so with this update you can simply set those variables like this:

$this->navigation('navigation')
    ->menu()
    ->setPartial('partial/yourPartial.phtml','yourModule')
    ->setPartialParams(array('variableName' =>$variable));

That's all.

[froschdesign]

👎
This is not needed, because:

• you can use menu options in partial view script (same for Breadcrumbs)
• or the view helper "Placeholder"

[weierophinney]

@froschdesign From reading the description, I think the reporter wants to be able to pass arbitrary parameters to the partial, not just menu options.

@Zyqsempai Please add unit tests, and also run ./vendor/bin/php-cs-fixer fix within your checkout before your next push.

[froschdesign]

@weierophinney
The description was not clear enough. Therefore, the second option: view helper "Placeholder".

[froschdesign]

@Zyqsempai

->setPartial('partial/yourPartial.phtml','yourModule')

There is no second parameter!

[froschdesign]

@Zyqsempai

Added possibility to set parameters(variables) for partial to populate in the view.

Can you provide a use case?

[Zyqsempai]

Hi , here is use case from my current project,
we have few view's and we need to show on them one navigation-partial but with some differences , for that we need to transmit simple flag(show or not) into this partial with this update you can simple do like this:
$this->navigation('navigation')
->menu()
->setPartial('partial/yourPartial.phtml','yourModule')
->setPartialParams(array('flag' => false));

[Zyqsempai]

Added unit test for setter and getter for PartialParams , also fixed all issues with php-cs-fixer.

[froschdesign]

for that we need to transmit simple flag(show or not)

Show or not? You can use the custom page properties:

$page1 = new Zend\Navigation\Page\Mvc();
$page1->header = true;

$page2 = new Zend\Navigation\Page\Mvc();
$page2->footer = true;

I'm not entirely against this PR, but I think there are too many options in the navigation view helpers.

->setPartial('partial/yourPartial.phtml','yourModule')

Again: there is no second parameter!

[weierophinney]

I like the general idea (passing variables to the partial). However, I'm not convinced by the implementation. Typically, I suspect I'd want to pass parameters to the partial on a case-by-case basis, at the time of invocation — in other words, I wouldn't want stateful parameters. As such, I'm wondering if this should warrant an alternate rendering method instead of additional helper properties — something like this:

echo $this->navigation()->menu()->renderPartialWithParams(['flag' => $flag]);

The signature would be:

public function renderPartialWithParams(array $params = [], $container = null, $partial = null)

I've provided a "sketch" if the idea here: weierophinney@8091297

@froschdesign — what do you think of this approach?

[froschdesign]

@weierophinney
Looks good to me. No extra options in the helper!

[weierophinney]

@Zyqsempai Do you want to take it from here? or would you like me to do a competing PR?

[Zyqsempai]

@weierophinney, What do you mean, "to take it from here"?
Regarding your idea i will be glad to help(if i can) to implement this functionality.

[Martin-P]

Personally I think the method name indicates how it should be implemented: renderPartialWithParams. Render a partial with parameters, thus adding the parameters to the method renderPartial.

The suggested implementation makes me think about methods like renderMenuWithContainer or renderMenuWithContainerAndOptions. This is (fortunately) not implemented that way and instead there is a single method renderMenu with optional parameters $container and $options. It makes more sense to me to use the same approach with renderPartial.

[weierophinney]

@Martin-P The problem is that renderPartial() has the following signature already:

public function renderPartial($container = null, $partial = null)

These are both null by default, and the method does some work to auto-determine them if they are not present. Considering the standard use case is to pass no arguments, adding the parameters as an option to the end makes the method unwieldy:

echo $this->navigation->menu()->renderPartial(null, null, ['flag' => $flag]);

This is why I suggested adding a new method, putting the argument at the start.

@Zyqsempai — by "take it from here," I was inviting you to re-factor your patch to follow the direction I was providing. :) I haven't written tests, as I just wanted to demonstrate how I'd approach it, so you'd need to:

• Merge my commit, or copy/paste it into your branch.
• Make similar changes for the Breadcrumbs helper.
• Write tests for each that validate that the parameters passed are being passed to the partial via the view model.

You can then push to this same pull request. I'd recommend starting from a fresh branch, and then force-pushing to the original branch on your github fork.

[Zyqsempai]

@weierophinney , Thank you for your invitation, i will start it tomorrow.

[Martin-P]

@weierophinney I understand your point, but isn't it more intuitive to keep the method more like the regular partial view helper? At this moment when I want to use renderPartial with the default container I already have to use null as the first parameter.

Looking at renderPartialWithParams I think most people will use it like this:

echo $this->navigation->menu()->renderPartialWithParams(
    ['flag' => $flag],
    null,
    'myPartial.phtml'
);

So perhaps it is better to ommit the $container completely here. We always have setContainer for this and don't really need it as a parameter(which only causes trouble). That way the method can be simplified:

public function renderPartialWithParams($partial = null, array $params = [])

And looking at this... I can't ignore the possibility to introduce a BC break instead of creating new methods and change the signature for renderPartial into the same signature as the partial view helper:

public function renderPartial($partial = null, array $params = [])

I am aware it is a BC break, but I think this is a better approach instead of creating fixes because of something that can possibly be called a design flaw?

[weierophinney]

@Zyqsempai I'm trying to keep the order of the arguments internally consistent in the helpers. Since the withPartial() method already has container and partial in that order, it makes sense to keep them in that order throughout. We can revisit the parameter order at a later date.

[froschdesign]

Please close this PR. It was merged with #8.

[froschdesign]

ping @weierophinney