latte n:if and n:foreach on same div behaviour

23 days ago

dkorpar
Member | 47
+
-1
-

For example

<ul>
    <li>item</li>
    <li n:if="!empty($rows)" n:foreach="$rows as $row">$row->id</li>
</ul>

will allways first run foreach. Which makes sense since then you can do:

<li n:if="$row->should_display" n:foreach="$rows as $row">$row->id</li>

but also is confusing developers…
I'm wondering would it be better to actually transform this in order which one is written first? if or foreach?
so this would be different

<ul>
    <li>item</li>
    <li n:if="!empty($rows)" n:foreach="$rows as $row">$row->id</li>
</ul>

from

<ul>
    <li>item</li>
    <li n:foreach="$rows as $row" n:if="!empty($rows)">$row->id</li>
</ul>

But then again this is also maybe not the best idea…
Few other developers that work with me and me already bumped into this and not really sure that this is solved in best way.
I'm not proposing anything just expect some disscussion :)

Last edited by dkorpar (2017-09-27 15:06)

23 days ago

David Grudl
founder | 6703
+
+2
-

This is intentional and the change would be a huge BC break, so it does not make sense to discuss about it :-)

For database queries is usually usable <li n:foreach="$rows as $row">$row->id</li>, when $rows can be null or false, you can use workaround <li n:foreach="$rows ?: [] as $row">$row->id</li> or <li n:foreach="(array) $rows as $row">$row->id</li>

14 days ago

dakur
Member | 7
+
0
-

I clash with this quite often, for example:

<div title="{$postFormat->name}" n:class="postsOverviewCarousel-item-image-postFormat, 'postsOverviewCarousel-item-image-postFormat-' . $postFormat->slug" n:foreach="$post->postFormats as $postFormat"></div>

$post is an instance of Article or News or StaticPage which are DTOs with common parent Post. But each of these children may declare its own properties. That's the case of postFormats – some of them have it, some do not. So $post->postFormats can exist but does not have to. And now n:if with n:foreach does not work for me.

@DavidGrudl As a new major version of Nette is in progress, when is better time to discuss it and make huge BCs then now?

Last edited by dakur (2017-10-06 10:36)

14 days ago

David Grudl
founder | 6703
+
+1
-

n:attributes are like HTML attributes: they occur only once, and the order does not matter.

14 days ago

dakur
Member | 7
+
0
-

I'm not saying it should matter. What about n:outer-if, does it make sense or it is more anti-pattern?

14 days ago

David Grudl
founder | 6703
+
0
-

I don't understand exactly what you would like to achieve and how one (@dkopar) or the second (@dakur) suggested solution would help you.

9 days ago

dakur
Member | 7
+
0
-

We use data containers in templates in our application which is kind of DTO/value object without getters and with public properties. Let's show two of them:

final class WorkshopDC extends PostDC
{
    public $isWorkshop = TRUE;
    public $id;
    public $postFormats;

    public function __construct(Workshop $workshop)
    {
        $this->id = $workshop->getId();

        $taxonomies = $workshop->getTaxonomies();
        $this->postFormats = TaxonomyUtils::convertTaxonomiesToDCs(Taxonomies::CATEGORY, $taxonomies); // Returns just an array of `TaxonomyTermDC` objects.
    }
}

final class NewsDC extends PostDC
{
    public $isNews = TRUE;
    public $id;

    public function __construct(News $news)
    {
        $this->id = $news->getId();
    }
}

Then, in a template, we need to render a collection of these data containers which are all children of abstract PostDC.

public function renderOverview()
{
    /** @var PostDC[] */
    $this->template->posts = $this->wordpressFacade->getAllPosts();
}

So I would do this:

<div n:foreach="$posts as $post">
    <div title="{$postFormat->name}" n:foreach="$post->postFormats as $postFormat" n:ifset="$post->postFormats"></div>
</div>

But it returns: Undefined property: App\Presenters\DataContainers\PostTypes\NewsDC::$postFormats rendering:

foreach ($post->postFormats as $postFormat) {
    if (isset($post->postFormats)) {
        // ...
    }
}

But I would expect or like to render it in opposite order:

if (isset($post->postFormats)) {
    foreach ($post->postFormats as $postFormat) {

        // ...
    }
}

Therefore, if interchangibility of n:foreach and n:if is a feature (which makes sense to me as well), I suggest n:outer-if which would render the above. Now, as a “workaround”, I have to use this:

<div n:foreach="$posts as $post">
    {ifset $post->postFormats}
        <div title="{$postFormat->name}" n:foreach="$post->postFormats as $postFormat"></div>
    {/ifset}
</div>

@DavidGrudl Is it more clear now?

Last edited by dakur (2017-10-11 14:48)

9 days ago

dakur
Member | 7
+
0
-

P.S.: I'm not saying, that n:outer-* attributes are good idea at all as there will be three types of all keywords then which could be a bit confusing, maybe. I just wanted to open a discussion about this problem I (we) have, finding a solution or coming to an end, that this is the best we can do with it.

Last edited by dakur (2017-10-11 14:52)

9 days ago

David Grudl
founder | 6703
+
+1
-

I see now. Just if/ifset works for the cycle elements, not the whole cycle. That it works this way was my decision, because I thought it is more useful in practice.

Of course, a new macro like outerif could be created, but I feel it brings unnecessary complexity. Prefix outer- is even more problematic.

Solution with {ifset $post->postFormats} is really good enough. It seems even more readable to me.

If you like expressed way, this should work too n:foreach="$post->postFormats ?? [] as $postFormat"