Skip to content

Commit

Permalink
Changes after review :
Browse files Browse the repository at this point in the history 
- remove the prepareLink method
- add missing throw tags
- code simplification / cleanup
  • Loading branch information
@HavokInspiration
HavokInspiration authored and yvesp committed Oct 6, 2016
1 parent 0c17f71 commit 978639d
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 63 deletions.
61 changes: 22 additions & 39 deletions src/View/Helper/BreadcrumbsHelper.php
View file
Expand Up @@ -43,7 +43,7 @@ class BreadcrumbsHelper extends Helper
protected $_defaultConfig = [
'templates' => [
'wrapper' => '<ul{{attrs}}>{{content}}</ul>',
'item' => '<li{{attrs}}><a href="{{link}}"{{innerAttrs}}>{{title}}</a></li>{{separator}}',
'item' => '<li{{attrs}}><a href="{{url}}"{{innerAttrs}}>{{title}}</a></li>{{separator}}',
'itemWithoutLink' => '<li{{attrs}}><span{{innerAttrs}}>{{title}}</span></li>{{separator}}',
'separator' => '<li{{attrs}}><span{{innerAttrs}}>{{custom}}{{separator}}</span></li>'
]
Expand All @@ -65,7 +65,7 @@ class BreadcrumbsHelper extends Helper
* - *title* The title of the crumb
* - *link* The link of the crumb. If not provided, no link will be made
* - *options* Options of the crumb. See description of params option of this method.
* @param string|array|null $link Link of the crumb. Either a string, an array of route params to pass to
* @param string|array|null $url URL of the crumb. Either a string, an array of route params to pass to
* Url::build() or null / empty if the crumb does not have a link
* @param array $options Array of options. These options will be used as attributes HTML attribute the crumb will
* be rendered in (a <li> tag by default). It accepts two special keys:
Expand All @@ -74,18 +74,17 @@ class BreadcrumbsHelper extends Helper
* - *templateVars*: Specific template vars in case you override the templates provided
* @return $this
*/
public function add($title, $link = null, array $options = [])
public function add($title, $url = null, array $options = [])
{
if (is_array($title)) {
foreach ($title as $crumb) {
$crumb = array_merge(['title' => '', 'link' => null, 'options' => []], $crumb);
$this->crumbs[] = $crumb;
$this->crumbs[] = $crumb + ['title' => '', 'url' => null, 'options' => []];
}

return $this;
}

$this->crumbs[] = ['title' => $title, 'link' => $link, 'options' => $options];
$this->crumbs[] = compact('title', 'url', 'options');

return $this;
}
Expand All @@ -94,7 +93,7 @@ public function add($title, $link = null, array $options = [])
* Prepend a crumb to the start of the queue.
*
* @param string $title Title of the crumb
* @param string|array|null $link Link of the crumb. Either a string, an array of route params to pass to
* @param string|array|null $url URL of the crumb. Either a string, an array of route params to pass to
* Url::build() or null / empty if the crumb does not have a link
* @param array $options Array of options. These options will be used as attributes HTML attribute the crumb will
* be rendered in (a <li> tag by default). It accepts two special keys:
Expand All @@ -103,9 +102,9 @@ public function add($title, $link = null, array $options = [])
* - *templateVars*: Specific template vars in case you override the templates provided
* @return $this
*/
public function prepend($title, $link = null, array $options = [])
public function prepend($title, $url = null, array $options = [])
{
array_unshift($this->crumbs, ['title' => $title, 'link' => $link, 'options' => $options]);
array_unshift($this->crumbs, ['title' => $title, 'url' => $url, 'options' => $options]);

return $this;
}
Expand All @@ -118,7 +117,7 @@ public function prepend($title, $link = null, array $options = [])
*
* @param int $index The index to insert at.
* @param string $title Title of the crumb
* @param string|array|null $link Link of the crumb. Either a string, an array of route params to pass to
* @param string|array|null $url URL of the crumb. Either a string, an array of route params to pass to
* Url::build() or null / empty if the crumb does not have a link
* @param array $options Array of options. These options will be used as attributes HTML attribute the crumb will
* be rendered in (a <li> tag by default). It accepts two special keys:
Expand All @@ -127,9 +126,9 @@ public function prepend($title, $link = null, array $options = [])
* - *templateVars*: Specific template vars in case you override the templates provided
* @return $this
*/
public function insertAt($index, $title, $link = null, array $options = [])
public function insertAt($index, $title, $url = null, array $options = [])
{
array_splice($this->crumbs, $index, 0, [['title' => $title, 'link' => $link, 'options' => $options]]);
array_splice($this->crumbs, $index, 0, [['title' => $title, 'url' => $url, 'options' => $options]]);

return $this;
}
Expand All @@ -142,21 +141,22 @@ public function insertAt($index, $title, $link = null, array $options = [])
*
* @param string $matchingTitle The title of the crumb you want to insert this one before
* @param string $title Title of the crumb
* @param string|array|null $link Link of the crumb. Either a string, an array of route params to pass to
* @param string|array|null $url URL of the crumb. Either a string, an array of route params to pass to
* Url::build() or null / empty if the crumb does not have a link
* @param array $options Array of options. These options will be used as attributes HTML attribute the crumb will
* be rendered in (a <li> tag by default). It accepts two special keys:
* - *innerAttrs*: An array that allows you to define attributes for the inner element of the crumb (by default, to
* the link)
* - *templateVars*: Specific template vars in case you override the templates provided
* @return $this
* @throws LogicException In case the matching crumb can not be found
*/
public function insertBefore($matchingTitle, $title, $link = null, array $options = [])
public function insertBefore($matchingTitle, $title, $url = null, array $options = [])
{
$key = $this->findCrumb($matchingTitle);

if ($key !== null) {
return $this->insertAt($key, $title, $link, $options);
return $this->insertAt($key, $title, $url, $options);
}
throw new LogicException(sprintf("No crumb matching '%s' could be found.", $matchingTitle));
}
Expand All @@ -169,21 +169,22 @@ public function insertBefore($matchingTitle, $title, $link = null, array $option
*
* @param string $matchingTitle The title of the crumb you want to insert this one after
* @param string $title Title of the crumb
* @param string|array|null $link Link of the crumb. Either a string, an array of route params to pass to
* @param string|array|null $url URL of the crumb. Either a string, an array of route params to pass to
* Url::build() or null / empty if the crumb does not have a link
* @param array $options Array of options. These options will be used as attributes HTML attribute the crumb will
* be rendered in (a <li> tag by default). It accepts two special keys:
* - *innerAttrs*: An array that allows you to define attributes for the inner element of the crumb (by default, to
* the link)
* - *templateVars*: Specific template vars in case you override the templates provided
* @return $this
* @throws LogicException In case the matching crumb can not be found
*/
public function insertAfter($matchingTitle, $title, $link = null, array $options = [])
public function insertAfter($matchingTitle, $title, $url = null, array $options = [])
{
$key = $this->findCrumb($matchingTitle);

if ($key !== null) {
return $this->insertAt($key + 1, $title, $link, $options);
return $this->insertAt($key + 1, $title, $url, $options);
}
throw new LogicException(sprintf("No crumb matching '%s' could be found.", $matchingTitle));
}
Expand Down Expand Up @@ -234,7 +235,7 @@ public function render(array $attributes = [], array $separator = [])

$crumbTrail = '';
foreach ($crumbs as $key => $crumb) {
$link = $this->prepareLink($crumb['link']);
$url = $crumb['url'] ? $this->Url->build($crumb['url']) : null;
$title = $crumb['title'];
$options = $crumb['options'];

Expand All @@ -249,12 +250,12 @@ public function render(array $attributes = [], array $separator = [])
'attrs' => $templater->formatAttributes($options, ['templateVars']),
'innerAttrs' => $templater->formatAttributes($optionsLink),
'title' => $title,
'link' => $link,
'url' => $url,
'separator' => '',
'templateVars' => isset($options['templateVars']) ? $options['templateVars'] : []
];

if (!($link)) {
if (!$url) {
$template = 'itemWithoutLink';
}

Expand Down Expand Up @@ -291,22 +292,4 @@ protected function findCrumb($title)

return null;
}

/**
* Prepare the URL for a specific `link` param of a crumb
*
* @param array|string|null $link If array, an array of Router url params
* If string, will be used as is
* If empty, will consider that there is no link
*
* @return null|string The URL of a crumb
*/
protected function prepareLink($link = null)
{
if (!$link) {
return null;
}

return $this->Url->build($link);
}
}
48 changes: 24 additions & 24 deletions tests/TestCase/View/Helper/BreadcrumbsHelperTest.php
View file
Expand Up @@ -54,14 +54,14 @@ public function testAdd()
$expected = [
[
'title' => 'Home',
'link' => '/',
'url' => '/',
'options' => [
'class' => 'first'
]
],
[
'title' => 'Some text',
'link' => [
'url' => [
'controller' => 'Some',
'action' => 'text'
],
Expand All @@ -82,12 +82,12 @@ public function testAddMultiple()
->add([
[
'title' => 'Home',
'link' => '/',
'url' => '/',
'options' => ['class' => 'first']
],
[
'title' => 'Some text',
'link' => ['controller' => 'Some', 'action' => 'text']
'url' => ['controller' => 'Some', 'action' => 'text']
],
[
'title' => 'Final',
Expand All @@ -98,22 +98,22 @@ public function testAddMultiple()
$expected = [
[
'title' => 'Home',
'link' => '/',
'url' => '/',
'options' => [
'class' => 'first'
]
],
[
'title' => 'Some text',
'link' => [
'url' => [
'controller' => 'Some',
'action' => 'text'
],
'options' => []
],
[
'title' => 'Final',
'link' => null,
'url' => null,
'options' => []
]
];
Expand All @@ -135,20 +135,20 @@ public function testPrepend()
$expected = [
[
'title' => 'The root',
'link' => '/root',
'url' => '/root',
'options' => ['data-name' => 'some-name']
],
[
'title' => 'Some text',
'link' => [
'url' => [
'controller' => 'Some',
'action' => 'text'
],
'options' => []
],
[
'title' => 'Home',
'link' => '/',
'url' => '/',
'options' => [
'class' => 'first'
]
Expand All @@ -173,31 +173,31 @@ public function testInsertAt()
$expected = [
[
'title' => 'Some text',
'link' => [
'url' => [
'controller' => 'Some',
'action' => 'text'
],
'options' => []
],
[
'title' => 'Insert At Again',
'link' => [
'url' => [
'controller' => 'Insert',
'action' => 'at_again'
],
'options' => []
],
[
'title' => 'Insert At',
'link' => [
'url' => [
'controller' => 'Insert',
'action' => 'at'
],
'options' => []
],
[
'title' => 'Home',
'link' => '/',
'url' => '/',
'options' => [
'class' => 'first'
]
Expand All @@ -222,25 +222,25 @@ public function testInsertBefore()
$expected = [
[
'title' => 'The super root',
'link' => null,
'url' => null,
'options' => []
],
[
'title' => 'The root',
'link' => '/root',
'url' => '/root',
'options' => ['data-name' => 'some-name']
],
[
'title' => 'Some text',
'link' => [
'url' => [
'controller' => 'Some',
'action' => 'text'
],
'options' => []
],
[
'title' => 'Home',
'link' => '/',
'url' => '/',
'options' => [
'class' => 'first'
]
Expand All @@ -265,25 +265,25 @@ public function testInsertAfter()
$expected = [
[
'title' => 'The root',
'link' => '/root',
'url' => '/root',
'options' => ['data-name' => 'some-name']
],
[
'title' => 'The less super root',
'link' => null,
'url' => null,
'options' => []
],
[
'title' => 'Some text',
'link' => [
'url' => [
'controller' => 'Some',
'action' => 'text'
],
'options' => []
],
[
'title' => 'Home',
'link' => '/',
'url' => '/',
'options' => [
'class' => 'first'
]
Expand Down Expand Up @@ -348,7 +348,7 @@ public function testRenderCustomTemplate()
$this->breadcrumbs = new BreadcrumbsHelper(new View(), [
'templates' => [
'wrapper' => '<ol itemtype="http://schema.org/BreadcrumbList"{{attrs}}>{{content}}</ol>',
'item' => '<li itemprop="itemListElement" itemtype="http://schema.org/ListItem"{{attrs}}><a itemtype="http://schema.org/Thing" itemprop="item" href="{{link}}"{{innerAttrs}}><span itemprop="name">{{title}}</span></a></li>',
'item' => '<li itemprop="itemListElement" itemtype="http://schema.org/ListItem"{{attrs}}><a itemtype="http://schema.org/Thing" itemprop="item" href="{{url}}"{{innerAttrs}}><span itemprop="name">{{title}}</span></a></li>',
'itemWithoutLink' => '<li itemprop="itemListElement" itemtype="http://schema.org/ListItem"{{attrs}}><span itemprop="name"{{innerAttrs}}>{{title}}</span></li>',
]
]);
Expand Down Expand Up @@ -388,7 +388,7 @@ public function testRenderCustomTemplateTemplateVars()
$this->breadcrumbs = new BreadcrumbsHelper(new View(), [
'templates' => [
'wrapper' => '{{thing}}<ol itemtype="http://schema.org/BreadcrumbList"{{attrs}}>{{content}}</ol>',
'item' => '<li itemprop="itemListElement" itemtype="http://schema.org/ListItem"{{attrs}}><a itemtype="http://schema.org/Thing" itemprop="item" href="{{link}}"{{innerAttrs}}><span itemprop="name">{{title}}</span></a>{{foo}}</li>',
'item' => '<li itemprop="itemListElement" itemtype="http://schema.org/ListItem"{{attrs}}><a itemtype="http://schema.org/Thing" itemprop="item" href="{{url}}"{{innerAttrs}}><span itemprop="name">{{title}}</span></a>{{foo}}</li>',
'itemWithoutLink' => '<li itemprop="itemListElement" itemtype="http://schema.org/ListItem"{{attrs}}><span itemprop="name"{{innerAttrs}}>{{title}}</span>{{barbaz}}</li>',
]
]);
Expand Down

0 comments on commit 978639d

Please sign in to comment.