taotoo Posted October 20, 2021 Share Posted October 20, 2021 I have a page tree structure like this: Exhibition - Exhibition 1 - Exhibition 2 - Exhibition 3 ... Work - Work 1 - Work 2 - Work 3 - Work 4 - Work 5 - Work 6 ... Works are assigned to Exhibitions via two page reference fields (using Connect Page Fields): exhibition_work in the Exhibition template work_exhibition in the Work template A given exhibition will include some of the Works, but not all. On the front end I have Exhibition pages (using the Exhibition template), each of which shows their Works in a grid (sorted manually with the page reference field UI). When a Work is clicked on it takes you to the Work page (Work template) for that particular Work. This is all working fine. *The issue* On the individual Work pages, I want to have prev/next arrows so that the user can navigate to adjacent Works. The arrows can't just link to the prev/next page in the tree, as that page may not appear in the relevant exhibition. I've got it working by using a URL parameter, and copying the relevant query from the Exhibition template, pasting it twice into the Work template and then fiddling with it (code below). Is there a more efficient way of doing this? (I've also tried the code from this post (thank you @Robin S) but I couldn't get the sort order to reflect the Page Reference field's manual order.) <!--Discover the current exhibition from the URL parameter--> <?php $url_exhibition = $input->get->text('exhibition'); ?> <!--Find all works in the current exhibition so we can find the current $key--> <?php foreach ($pages->find("template=exhibition, name=$url_exhibition") as $from_exhibition): ?> <?php foreach ($from_exhibition->exhibition_work as $key => $from_exhibition_work): ?> <!--Set $currentkey as the current work key--> <?php if ($from_exhibition_work->title == $page->title): ?> <?php $currentkey = $key; ?> <?php endif; ?> <?php endforeach; ?> <?php endforeach; ?> <!--Add +1 to current work key (so we can use it for the 'next' arrow)--> <?php $currentkeyplusone = $currentkey +1; ?> <!--Find all works in the current exhibition (again!) so we can isolate the next work for the forward arrow--> <?php foreach ($pages->find("template=exhibition, name=$url_exhibition") as $from_exhibition): ?> <?php foreach ($from_exhibition->exhibition_work as $key => $from_exhibition_work): ?> <!--Make a next arrow link--> <?php if ($key == $currentkeyplusone): ?> <a href="../<?php echo $from_exhibition_work->title . '/' . '?exhibition=exhibition1' ?>">NEXT</a> <?php endif; ?> <?php endforeach; ?> <?php endforeach; ?> Link to comment Share on other sites More sharing options...
taotoo Posted October 20, 2021 Author Share Posted October 20, 2021 I've made the code more concise, but not sure how to rearrange it to avoid the error message on each click of the Next link subsequent to the first. <!--Discover the current exhibition from the URL parameter--> <?php $url_exhibition = $input->get->text('exhibition'); ?> <!--Find all works in the current exhibition so we can find the current $key--> <?php $from_exhibition = $pages->get("template=exhibition, name=$url_exhibition"); ?> <?php foreach ($from_exhibition->exhibition_work as $key => $from_exhibition_work): ?> <!--Set $currentkey as the current work key--> <?php if ($from_exhibition_work->title == $page->title): ?> <?php $currentkey = $key; ?> <?php endif; ?> <!--Add +1 to current work key (so we can use it for the 'next' arrow)--> #59 <?php if ($key == $currentkey +1): ?> <!--Make a next arrow link--> <a href="../<?php echo $from_exhibition_work->title . '/' . '?exhibition=' . $url_exhibition ?>">NEXT</a> <?php endif; ?> <?php endforeach; ?> Link to comment Share on other sites More sharing options...
kongondo Posted October 20, 2021 Share Posted October 20, 2021 (edited) I struggle to read alternative PHP syntax but it seems to me you haven't defined $currentKey before the condition at line #55. If exhibition work title is not equal to the page title, you have no $currentKey, hence incrementing it at line 59 throws the error. Defining/initing it earlier, before line #55 ought to get rid of the error. E.g. $currentKey = 0; Not sure that your code will still work as intended though, so you'll need to test. Edited October 20, 2021 by kongondo clarification 1 Link to comment Share on other sites More sharing options...
taotoo Posted October 20, 2021 Author Share Posted October 20, 2021 12 minutes ago, kongondo said: I struggle to read alternative PHP syntax but it seems to me you haven't defined $currentKey outside the condition at line #55. If exhibition work title is not equal to the page title, you have no $currentKey, hence incrementing it at line 59 throws the error. Defining/initing it earlier, before line #55 ought to get rid of the error. E.g. $currentKey = 0; Not sure that your code will still work as intended though, so you'll need to test. You've nailed it, thank you very much Kongondo! Link to comment Share on other sites More sharing options...
kongondo Posted October 21, 2021 Share Posted October 21, 2021 9 hours ago, taotoo said: You've nailed it, thank you very much Kongondo! Glad you got it sorted. I have just noticed that you are using a GET input in a selector without sanitizing it first. Here: 11 hours ago, taotoo said: <!--Discover the current exhibition from the URL parameter--> <?php $url_exhibition = $input->get->text('exhibition'); ?> $url_exhibition is not sanitized and you immediately use it here: 11 hours ago, taotoo said: <!--Find all works in the current exhibition so we can find the current $key--> <?php $from_exhibition = $pages->get("template=exhibition, name=$url_exhibition"); ?> 1 Link to comment Share on other sites More sharing options...
taotoo Posted October 21, 2021 Author Share Posted October 21, 2021 8 hours ago, kongondo said: Glad you got it sorted. I have just noticed that you are using a GET input in a selector without sanitizing it first. Here: $url_exhibition is not sanitized and you immediately use it here: Thanks! I've sanitized it in the first couple of lines of the code shown below. Not sure if I'm doing it right or not... (I've also moved around the rest of the code in order to get the Prev link working reliably). <!--ARROWS--> <!--Discover the current exhibition from the URL parameter--> <?php $url_exhibition_unsanitized = $input->get->text('exhibition'); ?> <!--Sanitize the above--> <?php $url_exhibition = $sanitizer->name($url_exhibition_unsanitized); ?> <!--Set currentkey to avoid errors when not yet set--> <?php $currentkey = 99; ?> <!--Get relevant exhibition page--> <?php $from_exhibition = $pages->get("template=exhibition, name=$url_exhibition"); ?> <!--NEXT ARROW - This has to come before prev arrow, as $currentkey has to be updated first in order for prev arrow to work consistently--> <!--Loop through works in the current exhibition--> <?php foreach ($from_exhibition->exhibition_work as $key => $from_exhibition_work): ?> <!--Set $currentkey as the current work key--> <?php if ($from_exhibition_work->name == $page->name): ?> <?php $currentkey = $key; ?> <?php endif; ?> <!--Add +1 to current work key (so we can use it for the 'next' arrow)--> <?php if ($key == $currentkey +1): ?> <!--Make a next arrow link--> <a href="../<?php echo $from_exhibition_work->name . '/' . '?exhibition=' . $url_exhibition ?>">NEXT</a> <?php endif; ?> <?php endforeach; ?> <!--PREV ARROW--> <!--Again, loop through works in the current exhibition--> <?php foreach ($from_exhibition->exhibition_work as $key => $from_exhibition_work): ?> <!--Subtract -1 from current work key (so we can use it for the 'prev' arrow)--> <?php if ($key == $currentkey -1): ?> <!--Make a prev arrow link--> <a href="../<?php echo $from_exhibition_work->name . '/' . '?exhibition=' . $url_exhibition ?>">PREV</a> <?php endif; ?> <?php endforeach; ?> Link to comment Share on other sites More sharing options...
kongondo Posted October 21, 2021 Share Posted October 21, 2021 3 hours ago, taotoo said: <?php $url_exhibition = $sanitizer->name($url_exhibition_unsanitized); ?> Usually we sanitize at the earliest opportunity available. It is also good practice in case you forget or move things around. So, this is better and less verbose: <?php $url_exhibition = $sanitizer->name($input->get->text('exhibition')); ?> As for which sanitizer method to use, the docs are helpful, but you are probably aware of it already. Just nitpicking now... 3 hours ago, taotoo said: <?php $from_exhibition = $pages->get("template=exhibition, name=$url_exhibition"); ?> You could probably check if $url_exhibition is NOT empty before continuing with the subsequent code, i.e. the pages->get and the rest of the code ?. 1 Link to comment Share on other sites More sharing options...
taotoo Posted October 21, 2021 Author Share Posted October 21, 2021 47 minutes ago, kongondo said: Usually we sanitize at the earliest opportunity available. It is also good practice in case you forget or move things around. So, this is better and less verbose: <?php $url_exhibition = $sanitizer->name($input->get->text('exhibition')); ?> As for which sanitizer method to use, the docs are helpful, but you are probably aware of it already. That's great - I didn't know you could do it that way. Nice and neat. I picked 'name' as the sanitizer method as it seemed most appropriate, but will delve into it at some point. 47 minutes ago, kongondo said: Just nitpicking now... You could probably check if $url_exhibition is NOT empty before continuing with the subsequent code, i.e. the pages->get and the rest of the code ?. Good idea - it turns out it was showing an error if there was no URL parameter present. Thanks very much for all your help! Link to comment Share on other sites More sharing options...
Recommended Posts
Create an account or sign in to comment
You need to be a member in order to leave a comment
Create an account
Sign up for a new account in our community. It's easy!
Register a new accountSign in
Already have an account? Sign in here.
Sign In Now