<p>There are a lot of superfluous variables there. You don't need to keep track of whether there are restrictions, all you need to know is whether the current page and station are allowed to show the modal popup, based on whether they are included in the whitelist / blacklist, then combine those two settings.</p>
<p>Something like this should work:</p>
<pre class="lang-twig prettyprint-override"><code>{% set modalAllowedOnPages = popUpModal.showOnlyOnThesePages.ids() %}
{% set modalForbiddenOnStations = popUpModal.hideFromTheseLocations.ids() %}

{% set modalAllowedOnPage = modalAllowedOnPages ? (entry.id in modalAllowedOnPages) : true %}
{% set modalAllowedForStation = myStation.id in modalForbiddenOnStations %}

{% set showModal = modalAllowedOnPage and not modalAllowedForStation %}
</code></pre>
<ul>
<li>Note the ternary operator for <code>modalAllowedOnPage</code> – if the whitelist is empty, the modal is allowed on all pages.</li>
<li>Using <code>ids()</code> instead of <code>all()</code> to execute the queries is more efficient, because you don't need the entire <code>Entry</code> object, just the selected IDs.</li>
</ul>
<hr />
<p>Another way to write the second condition would be like this:</p>
<pre class="lang-twig prettyprint-override"><code>{% set modalAllowedForStation = popUpModal.hideFromTheseLocations.id(myStation.id).exists() %}
</code></pre>
<p>But I think this is a bit harder to understand, and it doesn't work for the first condition (because it doesn't work if no entries are selected there, which changes the behaviour).</p>

CC BY-SA 4.0
2023-04-21T14:41:42.400Z
Wow. That's awesome.  I'll give it a shot and let you know. 
Thank you! All of that is helpful.
Dwayne Jamieson
CC BY-SA 4.0
2023-04-21T15:07:00.073Z
I was able to use your code with only one change:
{% set modalForbiddenForStation = myStation.id in modalForbiddenOnStations %}

{% set showModal = modalAllowedOnPage and not modalForbiddenForStation %}
Dwayne Jamieson
CC BY-SA 4.0
2023-04-21T15:11:16.257Z
@DwayneJamieson Ah right, the second variable needs to be negated. I've updated my example code. If your question is solved, don't forget to mark the answer as accepted!
MoritzLost
CC BY-SA 4.0
2023-04-21T21:56:04.410Z
Done. Thanks. That one thing threw me off. That was awesome though. Much appreciated as I get better at this.
Dwayne Jamieson
CC BY-SA 4.0
2023-04-21T14:08:18.913Z
2023-04-21T15:10:07.227Z
2023-04-21T15:10:07.227Z
MoritzLost
<p><a href="https://i.sstatic.net/hIeKR.png" rel="nofollow noreferrer"><img src="https://i.sstatic.net/hIeKR.png" alt="enter image description here" /></a>This is my first post here. Is this a softball question?</p>
<p>How could I simplify the following code in twig?</p>
<p>I've placed the controls for having popUp Modal windows in Globals rather than put these settings on specific pages.</p>
<p>I also have the Google Maps plug-in which is managing the location settings based on ip.</p>
<p>The following code is functional as it is, but I don't like repeating if else statements like this if there's a cleaner way.</p>
<p>So how would others refactor this?</p>
<p>FYI: noPageLimitation and noGeoLimitation means that neither of the options are selected.</p>
<pre><code>{% set availablePage = true %}
{% set availableLocation = true %}
{% set noPageLimitation = true %}

{% if popUpModal.showOnlyOnThesePages|length %}
    {% set availablePages = popUpModal.showOnlyOnThesePages.all() %}
    {% if availablePages|contains('id', entry.id) %}
        {% set availablePage = true %}
    {% else %}
        {% set availablePage = false %}
    {% endif %}
    {% set noPageLimitation = false %}
{% else %}
    {% set noPageLimitation = true %}
{% endif %}
{% if popUpModal.hideFromTheseLocations|length %}
    {% set hiddenLocations = popUpModal.hideFromTheseLocations.all() %}
    {% if hiddenLocations|contains('id', myStation.id) %}
        {% set availableLocation = false %}
    {% else %}
        {% set availableLocation = true %}
    {% endif %}
    {% set noGeoLimitation = false %}
        {% else %}
    {% set noGeoLimitation = true %}
{% endif %}
{% if (noPageLimitation) and (noGeoLimitation) %}
    {% set showModal = true %}
{% elseif (noPageLimitation) and (availableLocation) %}
    {% set showModal = true %}
{% elseif (noGeoLimitation) and (availablePage) %}
    {% set showModal = true %}
{% elseif (availablePage) and (availableLocation) %}
    {% set showModal = true %}
{% else %}
    {% set showModal = false %}
{% endif %}
</code></pre>
<p>Any tips appreciated.</p>
<p>Thanks,
Dwayne</p>
<p><a href="https://i.sstatic.net/LekUp.png" rel="nofollow noreferrer"><img src="https://i.sstatic.net/LekUp.png" alt="enter image description here" /></a></p>

CC BY-SA 4.0
2023-04-21T13:35:59.343Z
Where are the other two variables coming from - `availablePage` and `availableLocation`?
James Smith
CC BY-SA 4.0
2023-04-21T13:46:42.607Z
They are there before all this code as well. I just didn't want to list them all. Actually, good point. I will post them in there.
Dwayne Jamieson
<conditional><globals>
Looking to simplify this if else. Any tips to make this simpler?
# Looking to simplify this if else. Any tips to make this simpler?

(Asked by: username_0 on 2023-04-21)

This is my first post here. Is this a softball question?
How could I simplify the following code in twig?
I've placed the controls for having popUp Modal windows in Globals rather than put these settings on specific pages.
I also have the Google Maps plug-in which is managing the location settings based on ip.
The following code is functional as it is, but I don't like repeating if else statements like this if there's a cleaner way.
So how would others refactor this?
FYI: noPageLimitation and noGeoLimitation means that neither of the options are selected.
{% set availablePage = true %}
{% set availableLocation = true %}
{% set noPageLimitation = true %}

{% if popUpModal.showOnlyOnThesePages|length %}
    {% set availablePages = popUpModal.showOnlyOnThesePages.all() %}
    {% if availablePages|contains('id', entry.id) %}
        {% set availablePage = true %}
    {% else %}
        {% set availablePage = false %}
    {% endif %}
    {% set noPageLimitation = false %}
{% else %}
    {% set noPageLimitation = true %}
{% endif %}
{% if popUpModal.hideFromTheseLocations|length %}
    {% set hiddenLocations = popUpModal.hideFromTheseLocations.all() %}
    {% if hiddenLocations|contains('id', myStation.id) %}
        {% set availableLocation = false %}
    {% else %}
        {% set availableLocation = true %}
    {% endif %}
    {% set noGeoLimitation = false %}
        {% else %}
    {% set noGeoLimitation = true %}
{% endif %}
{% if (noPageLimitation) and (noGeoLimitation) %}
    {% set showModal = true %}
{% elseif (noPageLimitation) and (availableLocation) %}
    {% set showModal = true %}
{% elseif (noGeoLimitation) and (availablePage) %}
    {% set showModal = true %}
{% elseif (availablePage) and (availableLocation) %}
    {% set showModal = true %}
{% else %}
    {% set showModal = false %}
{% endif %}

Any tips appreciated.
Thanks,
Dwayne

**Tags:** conditional, globals
> **Comments:**
> **username_1**: Where are the other two variables coming from - `availablePage` and `availableLocation`?  
> **username_0**: They are there before all this code as well. I just didn't want to list them all. Actually, good point. I will post them in there.  

---

## 1 answers

### Answer by username_2 on 2023-04-21. Score: 2 | Accepted answer

There are a lot of superfluous variables there. You don't need to keep track of whether there are restrictions, all you need to know is whether the current page and station are allowed to show the modal popup, based on whether they are included in the whitelist / blacklist, then combine those two settings.
Something like this should work:
{% set modalAllowedOnPages = popUpModal.showOnlyOnThesePages.ids() %}
{% set modalForbiddenOnStations = popUpModal.hideFromTheseLocations.ids() %}

{% set modalAllowedOnPage = modalAllowedOnPages ? (entry.id in modalAllowedOnPages) : true %}
{% set modalAllowedForStation = myStation.id in modalForbiddenOnStations %}

{% set showModal = modalAllowedOnPage and not modalAllowedForStation %}


Note the ternary operator for modalAllowedOnPage – if the whitelist is empty, the modal is allowed on all pages.
Using ids() instead of all() to execute the queries is more efficient, because you don't need the entire Entry object, just the selected IDs.


Another way to write the second condition would be like this:
{% set modalAllowedForStation = popUpModal.hideFromTheseLocations.id(myStation.id).exists() %}

But I think this is a bit harder to understand, and it doesn't work for the first condition (because it doesn't work if no entries are selected there, which changes the behaviour).

> **Comments:**
> **username_0**: Wow. That's awesome.  I'll give it a shot and let you know.  Thank you! All of that is helpful.  
> **username_0**: I was able to use your code with only one change: {% set modalForbiddenForStation = myStation.id in modalForbiddenOnStations %}  {% set showModal = modalAllowedOnPage and not modalForbiddenForStation %}  
> **username_2**: @DwayneJamieson Ah right, the second variable needs to be negated. I've updated my example code. If your question is solved, don't forget to mark the answer as accepted!  
> **username_0**: Done. Thanks. That one thing threw me off. That was awesome though. Much appreciated as I get better at this.  

---

