nav-child should be made site-agnostic

[PBruk]

Tagging @matt-bernhardt for code-review: this branch removes site-specific references, instead checking if menu has been assigned and splits postHead.php into 2 specific concerns: the header and navbar. Resolves #74.

[matt-bernhardt]

I need to ponder this change for a bit. I think I understand the change you're proposing, but want to make sure its the direction we want to head.

[PBruk]

totally makes sense...it's no small switcheroo that i am proposing.

On Thu, Jul 7, 2016, 10:45 AM Matthew Bernhardt notifications@github.com wrote:

I need to ponder this change for a bit. I think I understand the change you're proposing, but want to make sure its the direction we want to head.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MITLibraries/MITLibraries-child/pull/78#issuecomment-231099806, or mute the thread https://github.com/notifications/unsubscribe/ABv1Vfk4oJTh4tHR2_AwVI3-BVEGPoQUks5qTREggaJpZM4JG-Oq .

[matt-bernhardt]

After reviewing this, and the current state of the Child theme, I have a few questions for either @PBruk or @frrrances:

One of the big changes here is a shift in the approach to the header and nav elements. Currently, a page template calls one file, inc/postHead.php, which then renders both the header image (directly in inc/postHead.php and the navbar (in a sub-template, inc/nav-child.php). This PR proposes to have individual pages each call both partials. My questions about this are:

• How often do sites using the Child theme need one but not both pieces of content? Do sites ever need the masthead image and not the navbar, or use the navbar but not the masthead image?
• If it is ever the case that a site does not need both elements, how are content editors controlling whether the desired element appears / does not appear?
• How often do we anticipate rolling new sites out, and how much of a risk is it to have navigation hinge on a specifically-named menu existing? The PR looks like it automagically loads the child-nav menu on the referenced pages, provided such menu exists. Is that likely to trip up site builders? On a related note, are the site builders likely to only be part of the UX team, or might they be distributed around the organization?

I'm mildly concerned about hanging the navigation display off a specifically-named menu, but I'm slightly more concerned (not to say opposed) to requiring that each page template replace the same structure to conditionally load both elements:

<?php

get_template_part( 'inc/header-child' );

// Checks to see if menu has not been assigned.
if ( !has_nav_menu( 'child-nav' ) ) {

    wp_nav_menu( array(
        'theme_location' => 'child-nav',
        'fallback_cb' => false,
    ) );

    echo '' ;

} else {

    // Otherwise, if menu has been assigned, get navbar. -->
    get_template_part( 'inc/nav', 'child' );

}

Repeating code like this on N page templates (where N is currently 5, and may get larger over time) seems like a recipe for inconsistently-applied changes down the road. Might it be better to have the if/else structure contained within inc/nav-child.php, so the call from the pages would just be:

get_template_part( 'inc/header-child' );
get_template_part( 'inc/nav', 'child' );

?

[matt-bernhardt]

Almost a year later, I think it is time to close this and clean up our backlog of work. Philip, if you get a zombie notification for this, my apologies :-)