Add pico-pages-list plugin

[nliautaud]

No description provided.

[PhrozenByte]

Implementing a nested page hierarchy (i.e. a page tree) is a good idea, however, you really shouldn't replace the $pages array by a page tree. Other plugins don't expect this and will behave strange. You should provide the page tree using a complete separate variable. By the way, starting with Pico 2.0, the official method for "hiding" a page is by prefixing the filename with a _ - this feature will be part of Pico's core.

Please also note that there will be a official plugin to implement page trees with Pico 2.0 (see picocms/Pico#334). So, I'm not sure what we should do with this submission @smcdougall. By addressing the above change requests, @nliautaud basically creates a whole new plugin that will partially clash with a long planned official plugin for Pico 2.0. So we'll have to remove it from our website after Pico 2.0 has been released anyway. So, I think the best way handling this situation is to keep it as-is just in our wiki.

What do you guys think, what should we do?

[nliautaud]

Hi, thanks for the review(s).

Did I replace directly the Pico pages array somewhere ? I think I didn't get what you mean, but my PHP is rusty.

About what should be done with it, I didn't follow the development of Pico and for now I'm mainly updating my 4 years old plugins, so I let you decide what you think is best :)
Do you have a link to the will-be official plugin ?

That's great that there will be a way to hide pages in Pico core. Allowing to hide specific paths from the plugin output may be useful for managing separately this page list though. Actually a best way to do it would have been trough a twig function, to allow multiple concurrent outputs, ex {{ pages_list.exclude('foo/bar/', 'secrets/') }} or {{ pages_list.only('category/') }} (even if it's possible trough css for now). I may add an issue.

[PhrozenByte]

Did I replace directly the Pico pages array somewhere ?

Oh, my fault, I was wrong. You don't do this.

However, there's still the conflict with the not yet released official plugin. 😞 By the way, it might be a good idea to pass PicoPagesList::$pages as twig variable, not just the created HTML. 😉

Do you have a link to the will-be official plugin ?

I didn't create the repo yet, unfortunately don't have much time for Pico lately.

Allowing to hide specific paths from the plugin output may be useful for managing separately this page list though.

Sounds great! 👍

[nliautaud]

Hello, for info I've made some changes. README.md / commits

• default output with {{ PagesList.html }}
  • added output filtering trough twig instead of config with {{ PagesList.html(options) }}
  • added inclusive/exclusive filtering
  • added css utility classes
• exposed the nested array trough {{ PagesList.items }}, added example of recursive twig macro
• derived the page items from the Pico pages array, to give access to pages data in the nested array
• refactoring

---

A small thought :

For an official plugin, a cleaner interface allowing access to both a flat and a nested array, that can be both filtered and rendered, may be trough twig filters. Something like :

{{ pages }} // flat list of pages
{{ pages | nested }} // nested list of pages/dirs
{{ pages | html_tree }} // render the given array
{{ pages | only: 'foo/bar' }} // filter the given array
{{ pages | exclude: 'foo/bar', 'other' }} // filter the given array
// ex :
{{ pages | nested | exclude: 'foo/bar', 'other' | html_tree }}

I didn't implemented that as I'm not sure of the future of this thing 😃

[PhrozenByte]

Pico's official page tree plugin won't implement much more than building the page tree, everything else is up to the theme developer. So, extending your plugin's functionality as suggested might be a good idea to differentiate it from Pico's official plugin.

By the way: You shouldn't pass $this to Twig, since this allows plugin developers to call arbitrary public methods of both your plugin class and Pico (and not just .html and .items). Use multiple twig variables or a twig function, filter or tag instead. https://twig.symfony.com/doc/1.x/advanced.html is a good starting point for this 😃

[nliautaud]

Did the major changes, documented things and did a debug pass :) See PR above.

[PhrozenByte]

This definitely differentiates your plugin from Pico's official plugin, or to put it another way, your plugin is way more powerful than our official one and all of my concerns have been addressed. There are just some minor suggestions in nliautaud/pico-pages-list#5 (review), nothing significant. Great work @nliautaud! 👍

@smcdougall: As always, I neither tested the plugin nor checked the submission file, I was just checking the plugin's source code. What do you think about this one? 😃

[PhrozenByte]

Sorry for the unacceptable late response @nliautaud 😒

I just reviewed your plugin again and was about to finally accept your plugin, but noticed that you've updated it to Pico 2.0 in the meanwhile (good thing! 👍). I've opened nliautaud/pico-pages-list#18 with some small improvements/bug fixes. Please test the changes first. I'll accept your plugin immediately after this.

[PhrozenByte]

This PR got kinda abandoned... Time flies 🤦 😒 Sorry!

The plugin is pretty commonly used, so I merged it nevertheless there is still a improvement PR pending. Would love to see if you check nliautaud/pico-pages-list#18 and possibly merge it. Great work @nliautaud and thanks, you did a great work here! ❤️

[nliautaud]

No problem, same thing here. I won't be able to take a look and test the PR for a few months, so if you confirm that you checked it I'll merge it !

[PhrozenByte]

Just tested my PR (wow this is soooo old) and seems to work just fine 👍

[nliautaud]

Done 👍

[PhrozenByte]

There went something wrong, no idea how this could happen. 😒 Sorry! Please refer to nliautaud/pico-pages-list#25 @nliautaud