Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

introduce filtered pagination #376

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

DjThossi
Copy link

@DjThossi DjThossi commented May 12, 2018

I'd like to propose the following feature.

filtered pagination is providing an opportunity to generate an overview page only with blog posts which have a certain meta block provided. It is indifferent if it is part of a list or a simple string.

example
In blog post itself:

---
...
show: this
...
---

in overview page:

---
...
generator: pagination
pagination:
  max_per_page: 9
  provider: filtered.posts.show.this
...
---

On this overview page will now appear only entries with the meta information 'show: this'.

I hope you find this feature helpful and consider to merge it in.

$parts = explode('.', $matches[2]);
$name = $parts[0];
$expectedKey = $parts[1];
$expectedValue = $parts[2];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can simplified to this, what do you think?

list($name, $expectedKey, $expectedValue) = explode('.', $matches[2]);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm personally not the biggest fan of writing code like this because it reduces readability in my opinion.
I like the code more if it is written in a way that it is possible to read it like a book and not stacking up too many things into one line.
This is just personal taste. Anyways, I don't want to open a discussion on code style so I will just follow any advice.


$filteredData = array();
foreach ($data as $key => $page) {
if (is_string($page->meta()[$expectedKey]) && $page->meta()[$expectedKey] === $expectedValue) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is not it necessary to check the existence of the key, like this?

array_key_exists($expectedKey, $page->meta())

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As master is on PHP 7.2 now, both of these approaches could probably be simplified using the null coalesce operator. As an example:

$actualValue = $page->meta()[$expectedKey] ?? null;
if ($actualValue === $expectedValue) {
    ....
}

However, I'd caution against accepting that code example verbatim - I'm only speaking on a syntactical level, I'm not currently sure if the value null is an appropriate default to use there.

@beryllium
Copy link
Member

I'm not 100% sure what this would look like in the common use case. Could you provide a more detailed explanation of the goal and what the outputs/advantages would be?

I guess I'm not exactly sure what the advantage is here instead of the common usages of tags/categories. Or, to put it another way, I'm wondering if the taxonomies feature (used for tags and categories) could be leveraged to make this functionality more widely applicable.

@DjThossi
Copy link
Author

DjThossi commented May 31, 2018

Hey
I changed the code as recommended.

Here a bit more about the use case:
I had just one big travelblog where I have all my posts in chronological order.
The filtered function in Pagination helps me to generate additional pages where I now can combine all posts with a certain information like category or location. On top, this will not kill the chronological order of the post. Here an example for better understanding:
Here my general travel blog with all posts:

On top I build now sub overviews for my trip to OZ 2017

In the last source file, you can see now that I used the filtered option.

  provider: filtered.reiseblog.categories.2017-asia-oz

In all my blog posts regarding my trip to OZ you'll find this at the top:

categories:
  - de
  - Reiseblog
  - 2017-asia-oz

The function filtered() will now generate a list of entries which have a categories key with value 2017-asia-oz. Any other blog post will be ignored from list reiseblog will be ignored.

I hope now it is more clear what it's used for.

Copy link

@peter279k peter279k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using the short array syntax is better than array syntax 😄 :).


$data = $this->dataProviderManager->dataProvider($name)->provideData();
if (!count($data)) {
$data = array();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using the short array syntax? I think this short array syntax is better than array 😄.

$data = array();
}

$filteredData = array();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the same problem and I mention in the previous comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants