PhileCMS / Phile

A flat file CMS with a swappable parser and template engine.
https://philecms.github.io/
Other
257 stars 49 forks source link

resets pages keys after sorting to numerical values #106

Closed Schlaefer closed 10 years ago

Schlaefer commented 10 years ago

I want a simple next/previous page navigation in a blog setup with:

{% for page in pages %}
    {% if current_page == page %}
        {% if pages[loop.index0 + 1] is defined and pages[loop.index0 + 1].meta.date %}
            {% set divider = 1 %}
            <a href="{{ pages[loop.index0 + 1].url }}">
                {{ pages[loop.index0 + 1].title }}
            </a>
        {% endif %}
        {% if pages[loop.index0 - 1] is defined and pages[loop.index0 - 1].meta.date %}
            {% if divider %}
                –
            {% endif %}
            <a href="{{ pages[loop.index0 - 1].url }}">
                {{ pages[loop.index0 - 1].title }}
            </a>
        {% endif %}
    {% endif %}
{% endfor %}

in the template file.

Alas that's not possible because Phile has no next/previous page property and the pages keys aren't integers after the the sorting in Page.php and so loop.index + 1 does not work. Reseting the array keys after the sort should fix that and make the previous/next page accessible by loop.index+/-1.

james2doyle commented 10 years ago

Yeah this looks nice. I had a bad branch for next/prev support but it isnt great. Bump @NeoBlack to review

Schlaefer commented 10 years ago

I just found Phile\Model\Page\getPreviousPage and getNextPage in the 1.1 source …

james2doyle commented 10 years ago

Yeah but I don't think it works very well does it?

On Friday, August 15, 2014, Schlaefer notifications@github.com wrote:

I just found Phile\Model\Page\getPreviousPage and getNextPage in the 1.1 source …

— Reply to this email directly or view it on GitHub https://github.com/PhileCMS/Phile/pull/106#issuecomment-52293962.

James Doyle

t: @james2doyle https://twitter.com/james2doylew: ohdoylerules.com http://ohdoylerules.com

Schlaefer commented 10 years ago

From looking on the source code I would say the sorting was not applied, but I was lucky because I prefixed my content-files with a date string.

Hows about this implementation: a Page has access to a Repository it was created by. It can then ask that Repository, which manages the ordering about its neighbors.

Schlaefer commented 10 years ago

Hows about this implementation: a Page has access to a Repository it was created by. It can then ask that Repository, which manages the ordering about its neighbors.

On a second thought that's probably to complicated. Let's keep it simple and don't couple \Repository\Page and \Model\Page that way.

STOWouters commented 10 years ago

On another note about next/previous page pagination: my website has a prev/next navigation, I'm also using a Twig-trick, but it only works if you have a well-structured directory hierarchy, so it's not a general solution:

<ul class="pager">
  <li><a href="{{ base_url }}/blog/post/{{ current_page.url | split('/') | last - 1}}">&laquo;</a></li>
  <li><a href="{{ base_url }}/blog/">More posts</a></li>
  <li><a href="{{ base_url }}/blog/post/{{ current_page.url | split('/') | last + 1}}">&raquo;</a></li>
</ul>

Directory structure:

content/
|-- 404.md
|-- about.md
|-- blog
|   |-- index.md
|   `-- post
|       |-- 1.md
|       |-- 2.md
|       |-- 3.md
|       |-- 4.md
|       `-- 5.md
|-- index.md
|-- projects
|   |-- index.md
    `-- project
        |-- 1.md
        `-- 2.md

As you can see, it works because my posts are put in the blog/post directory and its filenames are actually integers (say, they are ID's actually).

The only caveat is that when you're on the last post, the next button will redirect you to a 404 page and also for the prev button on the first post likewise. I haven't found a clear way to check whether an url is valid or will redirect you to a 404 page.

I understand it's far from trivial to implement a prev/next pagination. It's a little ambiguous: do you want the previous/next page from a certain directory, or just want the previous/next page over all the markdown files? Even if you implement a next_page('from-this-dir/') Twig filter, things could get quickly messy. I would attempt to group the files together by which directory they are stored (root or subdirectory) first, then apply the sorting key for each groups. And keep a list of references to pages, sorted by the sorting key, without taking into account in which directory they are stored.

At least, I'm about writing a wiki entry for pagination using Twig-tricks, it's worth to see how others did it, most of the Phile sites are in fact blogs, so this kind of pagination is really useful.

Schlaefer commented 10 years ago

@Stijn-Flipper Maybe a page.folder attribute would be simpler solution? With that you can:

{% for page in pages %}
    {% if current_page.title == page.title %}
        {% if page.previousPage is defined and page.previousPage.folder == "project" %}
            <a href="{{ page.previousPage.url }}"> << Previous </a>
        {% endif %}

        {% if page.nextPage is defined and page.nextPage.folder == "project" %}
            <a href="{{ page.nextPage.url }}" > Next >> </a>
        {% endif %}
    {% endif %}
{% endfor %}

and don't need a complicated sorting behavior?

STOWouters commented 10 years ago

@Stijn-Flipper Maybe a page.folder attribute would be simpler solution?

That wouldn't work, because you check whether the previous page comes from the right folder, while I actually need the previous page in that folder.

For example:

sub/z.md  (most recent post)
sub/y.md
x.md
sub/w.md

If you're on sub/y and ask for the previous page, you would get x.md, which is not in the sub/ folder, so you would end up with a disabled button. Meanwhile there's still a sub/w.md which is actually the previous page of sub/y in the sub/ folder. Hope you understand the problem.

Anyway, I have my own pagination now, but it would be neat if there's a way to verify if an URL will redirect you to 404 or not, so I can disable the button on the right time.

Schlaefer commented 10 years ago

I updated the pull request. You can sort by multiple criteria now:

$config['pages_order']  = 'page.folder:desc meta.date:desc meta.title:desc';

So the code changes in this pull request are:

This is not the end-all solution but sorting for "folder" first (or any other meta/page tag) and "date" second should cover a lot of real world cases.

STOWouters commented 10 years ago

Thumbs up, this would cover my case too. :+1:

james2doyle commented 10 years ago

This sounds great. I will have to take a look later today. Thanks!

james2doyle commented 10 years ago

@NeoBlack has been in charge of the core classes. So I want to get his opinion on the merge first before it goes ahead.

james2doyle commented 10 years ago

Also this would need a version bump. I would say we can go to 1.2.

NeoBlack commented 10 years ago

the code looks good for me but I have not the time to test it. @james2doyle if you are fine with the code, please merge. also a new release 1.2.0 is ok for me.

STOWouters commented 10 years ago

@Schlaefer Are you sure it worked on your engine? Mine won't even get sorted on date alone, because the $options['pages_order'] is always empty (also: $this->settings is always empty), or is this a Phile issue?

STOWouters commented 10 years ago

I'm not sure if it was intended as a feature or it was actually a bug. @Schlaefer Can you take a look at PR #113 ?