Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the approach for {last} to be more like {else} on {foreach} loops, so that it would not be affected by {skipIf} #375

Closed
starfishpatkhoo opened this issue Sep 6, 2024 · 3 comments

Comments

@starfishpatkhoo
Copy link

starfishpatkhoo commented Sep 6, 2024

Version: 3.0.18

So I'm trying to have a simple index listing of pages.

{foreach $page_list as $slug => $page}
{skipIf ((! $user->is_admin()) && ($page->is_draft()))}
{first}<h1>List of Pages</h1><ul>{/first}
<li><a href={$slug}>{$page->title}</a></li>
{last}</ul>{/last}
{else}
<div class="alert">Oops! No Pages Found!</div>
{/foreach}

Now if it so happens that the last page in $page_list is a draft, and the user is not an admin, then the {last} never gets executed. Note that this also happens if we use {continueIf} instead.

A few observations:

  • If {first} came before the {skipIf} then of course {first} would get executed even if everything was skipped - which is expected and correct behaviour.
  • If we put {last} before the {skipIf} then it would work, but if the last page is a "visible" page, then the <ul> would appear before the actual link - which is also expected and correct behaviour, even if that's not what we want.

I know this is not really a Latte specific problem - it's the way loops are handled/done in the programming language. One could talk about setting a flag or counter to indicate if there is or isn't any data (and so the <ul> needs to be closed). Or maybe rejig the whole thing with more {if} statements instead. Or redesign the HTML so we do not depend on {first} and {last} to handle the <ul> tag.

But I'm just wondering if there is an easier, Latte-specific way (with nice tools like {skipIf}, {first}, {last} and {else}) to make this sort of thing happen?

I feel that for {last}, if we use a similar approach to handling {else}, we can execute {last} on the last iteration (even if it was {skipIf}-ed), as long as there was some data that wasn't {skipIf}-ed earlier.

Another approach, or another way to think about it, is to define a {header} and {footer} (for lack of a better word) that is executed before the first data in the loop (if there is data after any {skipIf}), and after the last data in the loop (if there was data). I think such a thing would be very useful for many common scenarios where we need to loop through stuff and display them.

{foreach $page_list as $slug => $page}
{header}<h1>List of Pages</h1><ul>{/header}
{footer}</ul>{/footer}
{skipIf ((! $user->is_admin()) && ($page->is_draft()))}
<li><a href={$slug}>{$page->title}</a></li>
{else}
<div class="alert">Oops! No Pages Found!</div>
{/foreach}

Ref #263 points to a similar issue, but affecting {sep} instead. I admit I think that is an even more complex problem to solve because one needs to look AHEAD in the loop to decide if there is or isn't another item after this (hence needing a {sep}). Maybe that requires an advanced version of implode().

@starfishpatkhoo
Copy link
Author

starfishpatkhoo commented Sep 7, 2024

Anyway, for anyone who happens on this issue, one way to achieve this is to add a flag (or counter) and not use {last}

{var bool $have_data = false}
{foreach $page_list as $slug => $page}
{skipIf ((! $user->is_admin()) && ($page->is_draft()))}
{first}<h1>List of Pages</h1><ul>{/first}
{var bool $have_data = true}
<li><a href={$slug}>{$page->title}</a></li>
{else}
<div class="alert">Oops! No Pages Found!</div>
{/foreach}
{if $have_data}</ul>{/if}

Be sure to initialize the flag OUTSIDE the foreach or else you will have an error if everything got skipIf-ed. Or use {if isset($have_data)} instead.

Another way is to redesign this to be more like the example in official documentation.

<h1>List of Pages</h1><ul>
{foreach $page_list as $slug => $page}
{skipIf ((! $user->is_admin()) && ($page->is_draft()))}
<li><a href={$slug}>{$page->title}</a></li>
{else}
<li>Oops! No Pages Found!</li>
{/foreach}
</ul>

But as you can see, the error message is now an entry in the <ul> list, and not an error box.

Maybe for the time being, this can be noted in the documentation about using {last} when there is a {skipIf} or {continueIf} in a loop so people know what to expect and use a workaround.

@dg
Copy link
Member

dg commented Oct 8, 2024

Opening tags in places where they are not closed (like <ul> in {first}<h1>List of Pages</h1><ul>{/first}) creates very confusing code, so avoid doing that. A much better solution is the second one.

To prevent the error message from being output inside the <ul> list, use the {try} & {rollback} tags:

{try}
	<h1>List of Pages</h1>
	<ul>
	{foreach $page_list as $slug => $page}
		{skipIf ((! $user->is_admin()) && ($page->is_draft()))}
		<li><a href={$slug}>{$page->title}</a></li>
	{else}
		{rollback}
	{/foreach}
	</ul>
{else}
	<div class="alert">Oops! No Pages Found!</div>
{/try}

@dg
Copy link
Member

dg commented Oct 8, 2024

It's not possible to modify the behavior of {last}, precisely due to the lookahead problem you mentioned.

@dg dg closed this as completed Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants