TASK: Add a category filter #2
Conversation
There was a problem hiding this comment.
Hey @obi12341,
Thanks for the change! I left some comments :)
| @@ -38,4 +38,13 @@ prototype(JohannesSteu.Neos.News:NewsFilter) < prototype(TYPO3.Neos:Content) { | |||
|
|
|||
| dateFilter = ${dateFilter} | |||
| } | |||
|
|
|||
| categoryFilter = ${q(site).find('[instanceof JohannesSteu.Neos.News:Category]').get()} | |||
There was a problem hiding this comment.
Should be named categories as it contains a set of catgeories, not a filter itself
| $nodeCategories = $node->getProperty("categories"); | ||
| foreach ($nodeCategories as $nodeCategory) { | ||
| /** @var $nodeCategory Node */ | ||
| if ($nodeCategory->getNodeData()->getProperty("uriPathSegment") == $categoryNode) { |
There was a problem hiding this comment.
The check for a categoryNode shouldn't be done with the uriPathSegement but with the nodetype and identifer or path. With this implementation you could call the categroyFilter with an non category node
There was a problem hiding this comment.
how can I pass to $arguments the complete object of the node? I tried, but it will only send the node path as string. So I have to get the node again and compare the node itself again. Isn't this way slower because I have to get the node from the repo first? And by the way the get parameter looks kind of ugly with the node path, looks more clear with just the uriPathSegement
| foreach($nodes as $node) { | ||
| /** @var $node NodeInterface */ | ||
| $nodeCategories = $node->getProperty("categories"); | ||
| foreach ($nodeCategories as $nodeCategory) { |
There was a problem hiding this comment.
Should be renamed to $categoryNode
| } else { | ||
| $nodesWithCategorySet = []; | ||
|
|
||
| $categoryNode = $arguments[0]; |
There was a problem hiding this comment.
$categoryNode should be checked if it is an instance of Category
| @@ -2,6 +2,9 @@ | |||
| {namespace ts=TYPO3\TypoScript\ViewHelpers} | |||
|
|
|||
| <f:section name="newsFilter"> | |||
| <f:if condition="{categoryFilter}"> | |||
| <ts:render path="categoryFilterTemplate" context="{categoryFilter:categoryFilter}" /> | |||
There was a problem hiding this comment.
context variables should als be named categories
| <ul class="news-categoryfilter"> | ||
| <li class="headline">Themen / Kategorien</li> | ||
| <li><neos:link.node node="{node}"><span class="news-categoryfilter__category">Alle</span></neos:link.node></li> |
| <li class="headline">Themen / Kategorien</li> | ||
| <li><neos:link.node node="{node}"><span class="news-categoryfilter__category">Alle</span></neos:link.node></li> | ||
| <f:for each="{categoryFilter}" as="filter"> |
There was a problem hiding this comment.
for each {categories} as category
| <f:for each="{categoryFilter}" as="filter"> | ||
| <li> | ||
| <neos:link.node node="{node}" arguments="{category: '{filter.properties.uriPathSegment}'}"><span class="news-categoryfilter__category">{filter.properties.title}</span></neos:link.node> |
There was a problem hiding this comment.
We shouldn't access .properties on a a node within fluid as it slows down rendering a lot
| templatePath = 'resource://JohannesSteu.Neos.News/Private/Templates/NodeTypes/NewsFilter.html' | ||
| sectionName = 'categoryFilter' | ||
|
|
||
| categoryFilter = ${categoryFilter} |
There was a problem hiding this comment.
Should be renamed to categories
| @@ -35,7 +35,9 @@ prototype(JohannesSteu.Neos.News:NewsList) < prototype(Flowpack.Listable:ListNod | |||
| [email protected] = ${q(value).count() > 0 && q(value).removeImportantNews(respectImportant).get()} | |||
|
|
|||
| # apply newsfilter if set | |||
| [email protected] = ${request.arguments.category != NULL ? q(value).filterByCategory(request.arguments.category).get() : value} | |||
There was a problem hiding this comment.
please prefix the argument with newsfilter as category is pretty much global and could also be used for something different
Added ability to filter by category.