antknight Posted May 11, 2013 Share Posted May 11, 2013 I have set up a site that has a news section available for all site editors to post news items. They can select one or more sections using the 'section' field which is a pageListSelectMultiple. I would like only the news items assigned a particular section to show up in that section. The code I am using on the section page works but I am unsure how it is doing it or whether it is the correct way. Any feedback is much appreciated. <?php $section = $page; $news = $pages->find("template=news-item, section={$section}, sort=-news_date, limit=5"); echo "<ul class='list'>"; foreach($news as $newsitem) { echo "<li class='list-item'>"; echo "<section>"; echo "<h5><a href='{$newsitem->url}'>{$newsitem->title}</a></h5>"; echo "<em>{$newsitem->news_date}</em>"; echo "</section>"; } echo "</ul>"; ?> Link to comment Share on other sites More sharing options...
adrian Posted May 11, 2013 Share Posted May 11, 2013 Looks good to me. The only feedback I have is that you could ditch the first line and replace $section in the second line with $page. 1 Link to comment Share on other sites More sharing options...
Macrura Posted May 11, 2013 Share Posted May 11, 2013 and aren't you missing the closing </li>; also not sure why you need the section tag within the <li> item, unless you are outputting the full news article text? 1 Link to comment Share on other sites More sharing options...
antknight Posted May 11, 2013 Author Share Posted May 11, 2013 Thanks guys. Quite right on all counts. The section tag has been left there from the many copy/paste/delete operations! Link to comment Share on other sites More sharing options...
arjen Posted May 11, 2013 Share Posted May 11, 2013 You can also ditch the ?>. You don't want any (invisible) characters after the >. I.e. trailing whitespace can lead to "headers already sent" errors. Those are notoriously hard to debug. Another thing you might consider is using a function. This way you can re-use the function in other templates. This might seem trivial or even more work, but in the long run you benefit when you want to change someting. function newsList($newsItems) { if (!count($newsItems)) return "There are no news-items found."; $out = ""; $out .= "<ul class='list'>"; foreach($news as $newsitem) { $out .= "<li class='list-item'>"; $out .= "<section>"; $out .= "<h5><a href='{$newsitem->url}'>{$newsitem->title}</a></h5>"; $out .= "<em>{$newsitem->news_date}</em>"; $out .= "</section>"; $out .= "</li>"; } return $out; } echo newsList($pages->find("template=news-item, section={$page}, sort=-news_date, limit=5")); Written in the browser, so not tested but I think you will get the idea. 2 Link to comment Share on other sites More sharing options...
kongondo Posted May 11, 2013 Share Posted May 11, 2013 And you can even ditch $news in this bit $news = $pages->find("template=news-item, section={$section}, sort=-news_date, limit=5"); ..... foreach($news as $newsitem) like this... (assuming you've ditched $section as well) foreach($pages->find("template=news-item, section={$page}, sort=-news_date, limit=5") as $newsitem) { //do your stuff } Power of nesting! Recently discovered this. It works a treat. For my testing, I have been leaving both styles in my code, commenting out the first of course, with notes about the second shorter version...for my learning purposes... //Aside: Btw, dear gurus, does the 2nd style affect performance? Other issues? If too long maybe can cause readability issues... 2 Link to comment Share on other sites More sharing options...
antknight Posted May 12, 2013 Author Share Posted May 12, 2013 I'm not sure I completely understand not having the ?> because the content following the PHP snippet is just plain HTML? All my templates look similar in structure to the head.inc included with the PW download. Otherwise brilliant ideas. I guess learning PHP and ProcessWire is a progressive thing and I will gradually pick up the shortcuts. I gave it a go but I think there is something missing from the code <?php /* Function Name: newsList Arguments: $newsItems Returns: */ function newsList($newsItems) { if (!count($newsItems)) return "There are no news-items found."; $out = ""; $out .= "<ul class='list'>"; foreach($news as $newsitem) { $out .= "<li class='list-item'>"; $out .= "<h5><a href='{$newsitem->url}'>{$newsitem->title}</a></h5>"; $out .= "<em>{$newsitem->news_date}</em>"; $out .= "</li>"; } $out .= "</ul>"; return $out; } <?php include 'functions.php'; echo newsList($pages->find("template=news-item, section={$page}, sort=-news_date, limit=5")); ?> Should $news be $newsItems? Link to comment Share on other sites More sharing options...
diogo Posted May 12, 2013 Share Posted May 12, 2013 It's good practice to leave the php tag unclosed in the end of the file because it's not needed at all, and even a simple space after it can cause errors. Link to comment Share on other sites More sharing options...
arjen Posted May 12, 2013 Share Posted May 12, 2013 You're absolutely right antkinght. Didn't test the code so thanks for pointing that out. For more arguments and reasoning on the closing tag do a little research on Google. 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