add next and previous buttons to learn pages

[johnkpaul]

This adds next and previous links to the content pages of learn.jquery.com. @nacin, I'd love some code review on my query in functions.php. It is no longer scoped to the parent id, because the menu_order is a single sequence across all parents.

[nacin]

Because menu_order is a single sequence, this is going to be much less performant as it needs to pull the details for all pages at once. So it probably makes sense to go with the original approach I pondered.

So I'm thinking two queries for posts_per_page => 1 and menu_order => $current - 1 and + 1.

Note that I think the menu_order query variable was added in 3.5, so you may need to use master to make it work.

[nacin]

Should work:

if ( version_compare( $wp_version, '3.5-alpha', '<' ) ) :
    add_filter( 'posts_where_paged', function( $where, $query ) {
        global $wpdb;
        if ( $menu_order = absint( $query->get( 'menu_order' ) ) )
            $where .= " AND $wpdb->posts.menu_order = " . $menu_order;
        return $where;
    }, 10, 2 );
endif;

If so, I'll add it to a new shims plugin.

[johnkpaul]

Thanks @nacin. This works fine for me. I have currently committed it into the local functions.php in the learn template, but once it is a plugin, I can remove that.

[ajpiano]

What's the good word here? Seems like this won't merge cleanly as of now.... Would like to be able to close jquery/learn.jquery.com#31

[johnkpaul]

Sorry, I didn't realize that this didn't merge cleanly anymore. I just merged with upstream/master and resolved the conflicts, but I don't know if that was the right way to handle this. Should I be making a new PR?

[ajpiano]

It appears that it can be automatically merged, but perhaps another pull without so many unrelated commits might be helpful at this point. I'll leave this up to @scottgonzalez @jzaefferer

[jzaefferer]

Don't listen to GitHub when it comes to mergability, also don't use the Green Button™.

In this case, the merge looks rather messy, so let's properly rebase this and squash fixup commits (including the merge commit). Usually an interactive rebase is the easiest way to do that: git rebase master -i. You may need to reset to the commit before that merge commit (dfb0d41), that would be git reset --hard dfb0d41^ (I think).

[johnkpaul]

@jzaefferer I interactively rebased and it looks like it should be a clean merge with only two commits. Let me know if this works.

[jzaefferer]

Okay, thats a lot better. Just tested this, the layout needs some improvements:

Could you improve that? While on it, can you squash your commits into a single one?

[johnkpaul]

@jzaefferer This should be fixed now and I squashed it to one commit.

[jzaefferer]

Nice. Found one more issue: The index page also get's these buttons, but just links to itself, twice.

[jzaefferer]

This doesn't really belong here, does it? I mean this entire file inside this PR.

[johnkpaul]

Yikes, sorry about that. I should have paid more attention.

[johnkpaul]

@jzaefferer What do you mean by the index page? When I go to http://dev.learn.jquery.com/about-jquery/, I get one single link which correctly goes to the first article in that group. I have no content in the homepage at all. I assumed that it was using a different template.

Is this not what I should see? I just grunt wordpress-deployd this morning.

[jzaefferer]

There should be something:

[johnkpaul]

😮 Hmm, I have to figure that out then. Maybe something is wrong with my wordpress instance.

[ajpiano]

@buritica and I are working on the index page right now. we just updated web-base-template last night to have the static index page plugin last night in 343f544, and will be pushing an index page shortly. i was experiencing the same issue as @johnkpaul with the main index not showing until we added the plugin.

[ajpiano]

This was due to a bad choice in grunt-jquery-content that I made in assigning posts draft status if they weren't in order.yml. The index page will be display normally if you pull learn.jquery.com master, npm install, and probably truncate your wp_5_posts and wp_5_postmeta tables. That said, the index page is going to need its own template, but that's a separate issue

[johnkpaul]

Thanks @ajpiano. After following those instructions, I do see the index page. I can add something to the page.php template to hide this, specifically for the index page. But if it is going to be a new template anyway, I don't know if I need to worry about it. I don't see any other pages that shouldn't have this. /cc @jzaefferer

[ajpiano]

I would say not to worry about it, the index page is going to have its own template (cc @buritica)

[buritica]

Yes, Its going to be a different template. I'm pushing that out today.

[jzaefferer]

Alright, landed!