It would be nice to omit pages from prev/next if :page-index nil

[seancorfield]

I'm using Cryogen for clojure-doc.org and I have :sidebar-omit? true in pages I don't want in the RH sidebar since I can modify base.html to omit them:

                    {% for page in sidebar-pages %}{% if not page.sidebar-omit? %}
                    <li><a href="{{page.uri}}">{{page.title}}</a></li>
                    {% endif %}{% endfor %}

But I can't find a way to omit pages from the prev/next navigation at the bottom of pages since this seems to be managed via sort-by :page-index -- it would be nice to have nil page index values filtered out so pages could easily be excluded from the prev/next sequence.

If there is a documented, supported way to do this already and I just missed it, feel free to point me at the docs!

[seancorfield]

I modified page.html to check for page.prev.page-index which allows me to omit :page-index:

    <div id="prev-next">
        {% if all page.prev page.prev.page-index %}
        <a href="{{page.prev.uri}}">&laquo; {{page.prev.title}}</a>
        {% endif %}
        {% if all page.prev page.prev.page-index page.next %}
        ||
        {% endif %}
        {% if page.next %}
        <a href="{{page.next.uri}}">{{page.next.title}} &raquo;</a>
        {% endif %}
    </div>

[yogthos]

That sounds reasonable, any chance you'd be up to do a PR for the feature? :)

[seancorfield]

I'll have to dig a bit further into the code to see what other impacts there might be in having :page-index nil (effectively) and I guess it could potentially be a breaking change for people who don't have :page-index everywhere if those pages suddenly disappeared from the prev/next index?

Perhaps just documenting the workaround I figured out (checking page.prev.page-index in the nav code in page.html) would be useful enough?

[yogthos]

One option could be to feature flag this in the config, so the behavior could be enabled conditionally by the user. That would address the concern in terms of regressions. I'm ok just to document this as well if you find it's sufficiently ergonomic.

[seancorfield]

Having experimented more with this, I think it does need to be a feature in the core project since my workaround shown above does still lead to some weird edge cases: while it suppresses "prev" nav for those "hidden" sub-pages, the "next" nav for them is not predictable and will link to other "hidden" sub-pages in some cases.

I haven't had time to dig deeper into what code changes might be needed to really make this work properly (I think just filtering out pages based on :page-index is all it would take) but I don't have a sense of whether this needs to be a specific feature flag in the config or not, nor what impact it would have on the documentation.

I'll continue to think about it as I work on https://clojure-doc.org

[yogthos]

That makes sense, and thinking bit more about it, would probably be fine if this was the default behavior. Since there's really no good way to decide what to do with a page that has a nil :page-index, omitting it would make sense in that case. Could probably just add a note under the doc for :page-index regarding the behavior.

[seancorfield]

Does that mean you are less concerned about this treatment -- hiding pages with no :page-index from the prev/next navigation -- being viewed as a "breaking" change?

[yogthos]

Yeah, having thought about it a bit, it seems to make sense to explicitly require :page-index for navigation. So, I'd say it would be fine to make this a breaking change.