OllieMackJames Posted May 30, 2013 Share Posted May 30, 2013 I have the following structure p1 -p1page2 -p1page3 -p1page4 p2 -p2page2 -p2page3 -p2page4 etc Now at the bottom of each article I want a link to the next article. So it should go like this: p1 links to p1page2 links to p1page3 links to p1page4 links to p2 links to p2page2 links to p2page3 etc. etc. I am using: <a href="<?php echo $page->next->url ?>"><?php echo $page->next->title ?></a> But that only gives me a next page on the same level, it does not traverse up or down the tree. What code do I need for this to traverse down when needed, then horizontal and then back up again. Thanks! Link to comment Share on other sites More sharing options...
teppo Posted May 30, 2013 Share Posted May 30, 2013 Edit: oh, wait, sorry -- completely misread your question Link to comment Share on other sites More sharing options...
horst Posted May 30, 2013 Share Posted May 30, 2013 p1 -p1page2 -p1page3 -p1page4 p2 -p2page2 -p2page3 -p2page4 ... What code do I need for this to traverse down when needed, then horizontal and then back up again. I think you first have to check where you are in the tree. If you have different templates for the pages p and subpages this is easy, but if you have not, you have to check for parents and childrens. Something like: if( count($page->children) > 0 ) { $url = $page->children->first()->url; } this is true if you are on p1 or p2, then you want to go to the first child-page. If the current page has no children, you want to go to the next page, but you also have to check if there is a next page or if you are allready on the last one: if( 0 != $page->next->id ) { $url = $page->next->url; } else { $url = $page->parents->next->url; } If it's the last one you want go to the next sibling of your current parent. But you also have to check if your current parent is the last one and there is no next one too: if( 0 != $page->next->id ) { $url = $page->next->url; } elseif( 0 != $page->parents->next->id ) { $url = $page->parents->next->url; } else { // do something when on last page, hmm, - yeah ok: $url = $page->parents->first()->url; } So, this is written in the browser and not tested, but something like this will work. 4 Link to comment Share on other sites More sharing options...
teppo Posted May 31, 2013 Share Posted May 31, 2013 Minor addition to above: $page->parent->first()->url should be $page->parents->first()->url.. or just $page->parent->url, which already returns first parent. Also: you could use $page->child instead of $page->children->first() to get first child Other than that, this seems to work properly. Now, I'm sorry for taking this thread a bit off track, but there's something strange going on here I'd like to point out: I was actually going to suggest a very similar approach, but after testing it ran into a problem that kept crashing whole Apache. Seems to be somehow tied to recursive function in sitemap template of default site profile and also happens with solution provided above. Once I comment out sitemapListPage() call in template file everything works properly, with it in place Apache keeps dying on me. Just for reference, code I was going with for the original problem was this: $next = ($page->numChildren) ? $page->child : $page->next; if ($next instanceof NullPage) $next = $page->parent->next; echo ($next instanceof NullPage) ? "This is the end" : "<a href='{$next->url}'>{$next->url}</a>"; After using either my approach or the one @horst suggested above, this seems to cause massive headache to Apache: function sitemapListPage($page) { echo "<li><a href='{$page->url}'>{$page->title}</a> "; if($page->numChildren) { echo "<ul>"; foreach($page->children as $child) sitemapListPage($child); echo "</ul>"; } echo "</li>"; } sitemapListPage($pages->get("/")); What am I missing here? 2 Link to comment Share on other sites More sharing options...
OllieMackJames Posted May 31, 2013 Author Share Posted May 31, 2013 Thanks horst and teppo! I went with: $next = ($page->numChildren) ? $page->child : $page->next; if ($next instanceof NullPage) $next = $page->parent->next; echo ($next instanceof NullPage) ? "This is the end" : "<a href='{$next->url}'>{$next->url}</a>"; and that just seems to work fine. Teppo, i do not understand the trouble you mentioned but would sure like to know what not to do so apache will not come crashing down... Other question I have with this solution is whether this will slow the site down considerably or not, because now for every page rendered I am probably asking processwire to check the tree first before rendering. Currently on the site I am using this on, there is a couple hundred pages, I am using procache, but still I would like to know how this affects speed. tnks! Link to comment Share on other sites More sharing options...
teppo Posted May 31, 2013 Share Posted May 31, 2013 Other question I have with this solution is whether this will slow the site down considerably or not, because now for every page rendered I am probably asking processwire to check the tree first before rendering. Currently on the site I am using this on, there is a couple hundred pages, I am using procache, but still I would like to know how this affects speed. There is a warning about next() and prev() in Page.php regarding larger amount of siblings: * Be careful with this function when the page has a lot of siblings. It has to load them all, so this function is best * avoided at large scale, unless you provide your own already-reduced siblings list (like from pagination) How big a problem this will cause depends on how many sibling pages you've going to have in each branch there (and the number of branches, of course, if there'll be a lot of those.) I tried to implement next and prev on a site that, at that time, had around 1500 siblings in each branch and was growing fast. That resulted in significant speed problems for each page load. I didn't have caching on at the time, though -- that definitely helps a lot. With caching enabled PW only has to build each page as a static HTML file once. Even if it's a bit slow, it should only affect first page view of each page in that branch. On the other hand, if content is updated very regularly (which I kind of doubt would be a problem here) this would of course diminish the benefit of caching a bit. All in all, I'd suggest that you implement this, do some tests regarding speed and see for yourself if it's going to be a problem. If possible, add some extra dummy pages to see how things will change once your site grows. 1 Link to comment Share on other sites More sharing options...
Wanze Posted May 31, 2013 Share Posted May 31, 2013 Hi guys I don't think that performance is a problem with this code. Teppo, with your solution you are only checking for the first child or next sibling. Pw does not load more pages than one. But as you mentioned, be careful when using next() / prev() / children() methods, as you can get lots of children loaded in memory. Note that next() != next Edit: Sorry, wrong thinking. See teppo's post below Link to comment Share on other sites More sharing options...
teppo Posted May 31, 2013 Share Posted May 31, 2013 @Wanze: are you sure about that? Taking a look at get method in Page.php (which, in turn, is called by magic method __get): public function get($key) { $value = null; switch($key) { ... case 'next': ... $value = $this->{$key}(); break; ... Sorry for all the dots there, but anyway: as far as I can see, actually next equals next() unless PW has already "cached" it's return value as a property, which shouldn't be the case here. And I don't see why above warnings wouldn't apply here either Link to comment Share on other sites More sharing options...
Soma Posted May 31, 2013 Share Posted May 31, 2013 next == next() children == children() parent == parent() ... and so forth, PW doesn't care here both are covered. Link to comment Share on other sites More sharing options...
Wanze Posted May 31, 2013 Share Posted May 31, 2013 @teppo Sorry you're right! I just looked at the method that is then returning the next child, PageTraversal::next() static public function next(Page $page, $selector = '', PageArray $siblings = null) { if(is_object($selector) && $selector instanceof PageArray) { // backwards compatible to when $siblings was first argument $siblings = $selector; $selector = ''; } if(is_null($siblings)) $siblings = $page->parent->children(); else if(!$siblings->has($page)) $siblings->prepend($page); $next = $page; do { $next = $siblings->getNext($next); if(!strlen($selector) || !$next || $next->matches($selector)) break; } while($next && $next->id); if(is_null($next)) $next = new NullPage(); return $next; } In our case $siblings is null. So the method above loads all the siblings to return only one!! Hmm... Thinking the next/prev/child calls could be optimized with a database query? Link to comment Share on other sites More sharing options...
Wanze Posted May 31, 2013 Share Posted May 31, 2013 Wouldn't something like this be insanly faster when having lot of pages for the next/prev calls? // in Page.php switch ($key) { case 'next': // I'm sure there's a more elegant way... no time to go trough mysqli docs $p = wire('db')->query("SELECT id FROM pages WHERE parent_id = {$this->parent} AND sort > {$this->sort} LIMIT1")->fetch_row(); if ($p) { $id = $p[0]; return wire('pages')->get($id); } return new NullPage(); break; } Just did a quick test: Calling next on a page with 87941 siblings: Current implementation: Loads forever... i stopped :-P With sample code above: in some milli seconds the next sibling was loaded Link to comment Share on other sites More sharing options...
Soma Posted May 31, 2013 Share Posted May 31, 2013 And the sorting and selectors? As you can also do next('selector'). And next() also is for in memory (PageArray) usage not only direct query. I'm sure Ryan has reasons Edit: but if performance matter you could create your own next() methods. Link to comment Share on other sites More sharing options...
Wanze Posted May 31, 2013 Share Posted May 31, 2013 If you are calling $page->next the magic getter is routing you forward to the $page->next() method. If you call $page->next('selector') you're calling the method explicit, so this won't get through the magic getter. In the first case you or by calling ->next() without arguments you want the next page (sort+1). But you're right.. I'm sure ryan has good reasons for the current implementation Edit: But still it seems not optimal that if you want to load the next/prev sibling, the current implementation loads all children under the parent... to return then only one Link to comment Share on other sites More sharing options...
Soma Posted May 31, 2013 Share Posted May 31, 2013 And does you method work with PageArray? Link to comment Share on other sites More sharing options...
Wanze Posted May 31, 2013 Share Posted May 31, 2013 And does you method work with PageArray? I don't understand what do you mean with PageArray? getNext()? Link to comment Share on other sites More sharing options...
Soma Posted May 31, 2013 Share Posted May 31, 2013 $page->next($pageArray); As I said Ryan must have reasons, but I don't know really and haven't analysed all those traversal methods just thinks it isn't that trivial. Link to comment Share on other sites More sharing options...
horst Posted May 31, 2013 Share Posted May 31, 2013 Also as a beginner and I'm not sure if I understand Wanze right, but maybe he means that it should be different handling for $page->next and $page->next() $page->next() handles as it does actually but $page->next should be optimized for the cases with mass siblings. so, conclusion: next == next() ?? right ?? or not ?? 1 Link to comment Share on other sites More sharing options...
Wanze Posted May 31, 2013 Share Posted May 31, 2013 @horst Yeah! Currently, when calling $page->next Pw calls $page->next() which then loads all siblings under the parent of $page. This is different from calling $page->next('selector') or $page->next($pageArray) - because you always want the next child in the tree (sort + 1). Therefore, I think that ->next (and also ->next()) could be optimized. @Soma $page->next($pageArray) won't execute my db query as this is also a direct call to the method I'm sure there are other side-effects I'm missing and there's a good reason for the current implementation. But interesting disscusion anyway, sorry for messing up this thread! 3 Link to comment Share on other sites More sharing options...
OllieMackJames Posted May 31, 2013 Author Share Posted May 31, 2013 @Wanze no problem at all to mess up my thread I got an answer and if Ryan chimes in and your thinking leads to a further improvement, even better still! PW rocks! And the community as well! Link to comment Share on other sites More sharing options...
ryan Posted June 2, 2013 Share Posted June 2, 2013 The reason that next (and prev) can't be optimized in that way is that it assumes that the sort field is "sort". Typically when you've got a large quantity of pages, they have some other sort, like date or title or who knows what. So that "sort" field certainly could be directly queried, but it wouldn't mean anything unless the page was set for manually sorted children. Manually sorted children is the only situation under which PW can keep that "sort" field up-to-date. If you've got thousands of children, chances are you aren't manually sorting them. You can get around the scalability consideration just by determining the 'prev' or 'next' using criteria specific to your situation. For instance, lets say your pages were sorted by a field named 'date': $date = $page->getUnformatted('date'); $next = $page->parent->child("date>$date, sort=date"); $prev = $page->parent->child("date<$date, sort=-date"); 3 Link to comment Share on other sites More sharing options...
Wanze Posted June 2, 2013 Share Posted June 2, 2013 Of course... this is really a useful snippet - thanks ryan! And all with 2 lines of code... I'm still overcomplicating things - Pw rocks! 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