As long as the implementation is *valid*, then we tend to give as much leeway as we can to the developer, with respect to design intent.<div><br></div><div>In this case, as long as the functional code itself is valid and correct, I would simply require the developer to add a description of the Nav Menu behavior to the Theme documentation, so that the user is aware that only the "Home" link displays if no Nav Menu is applied to the Theme Location.</div>
<div><br></div><div>(Note that it would be equally valid for the developer to fallback to *no menu displaying* if none is applied to the Theme Location - again, the key is that the behavior is documented, so that the end user knows what to expect.)</div>
<div><br></div><div>Chip<br><br><div class="gmail_quote">On Mon, Aug 1, 2011 at 7:56 AM, Mario Peshev <span dir="ltr"><<a href="mailto:mario@peshev.net">mario@peshev.net</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Hi there,<div><br>I'm reviewing the inLine theme from the queue (number 4584, link - <a href="http://themes.trac.wordpress.org/ticket/4584" target="_blank">http://themes.trac.wordpress.org/ticket/4584</a> ). I'm pretty positive about the code quality of the theme, the admin settings and the behavior based on the demo content. Theme checker plugins throw no exceptions, post formats are defined appropriately (except aside which shows title here) and almost everything looks great to me. The theme is applying additional hooks and the code is module-based and commented as well.</div>
<div><br></div><div>My only serious remark is that by default the theme top menu shows "Home" only. Not a standard menu, not the fallback page menu that WordPress applies by default. Just the Home link.</div><div>
<br>After code inspection I found that code snippet which expects a primary location for custom menu even if none is defined:</div><div><br></div><div><br></div><div><div>function inline_do_header_nav() {</div><div><span style="white-space:pre-wrap"> </span>// Create the primary nav menu</div>
<div><span style="white-space:pre-wrap"> </span>$inline_primary_nav = wp_nav_menu( array( 'theme_location' => 'primary', 'sort_column' => 'menu_order', 'container_id' => 'primary', 'container_class' => 'menu-header', 'echo' => 'false', 'fallback_cb' => 'inline_fallback_primary_nav' ) );</div>
<div><span style="white-space:pre-wrap"> </span></div><div><span style="white-space:pre-wrap"> </span>// Display the primary nav menu only if it is set</div><div><span style="white-space:pre-wrap"> </span>if ( $inline_primary_nav ) {</div>
<div><span style="white-space:pre-wrap"> </span>echo $inline_primary_nav;</div><div><span style="white-space:pre-wrap"> </span>}<span style="white-space:pre-wrap"> </span></div>
<div>}</div><div><br></div><div><br></div><div>I assume this is against the standard rules - or maybe it isn't? </div><div><br>If this expectation is allowed, I would suggest 'approved' status for the theme but I find it inappropriate for the fallback function to list 'Home' only when I have 12 pages on my demo site. </div>
<div><br>Thanks for the help,</div><br>Mario Peshev<br>freelance software developer/trainer<br><a href="http://www.linkedin.com/in/mpeshev" target="_blank">http://www.linkedin.com/in/mpeshev</a><br><a href="http://peshev.net/blog" target="_blank">http://peshev.net/blog</a><br>
<br>
</div>
<br>_______________________________________________<br>
theme-reviewers mailing list<br>
<a href="mailto:theme-reviewers@lists.wordpress.org">theme-reviewers@lists.wordpress.org</a><br>
<a href="http://lists.wordpress.org/mailman/listinfo/theme-reviewers" target="_blank">http://lists.wordpress.org/mailman/listinfo/theme-reviewers</a><br>
<br></blockquote></div><br></div>