Front-end pagination support #449
Conversation
|
Test this PR in WordPress Playground. |
|
I've had a look this morning and was able to do some shenanigans with input variables. These are bad-faith user inputs. pagination-inputs.mp4I have other thoughts about labels/UX that I can return to later in the day. @chriszarate, could you also stub out some documentation on this? Looking through the code, I think I know how it works, but it would be helpful for my review if we had a bit of documentation for me to check against. I'm also fine hearing that it's too early for that. |
I've opened an issue for this (VIPCMS-357); these inputs are generic and provided for input variables of any type (not just pagination input variables). This PR does not intersect with these inputs at all. |
I've updated the existing documentation for how pagination variables are defined and they are complete now. We don't have any documentation for the editor experience or the front-end render experience, so I haven't attempted any scoped to pagination. But I can if you think they should exist standalone. |
Great! I didn't bother to check the current documentation to see what was there, so I missed that we already had it. Sorry about that. I opened a Draft PR with some suggestions, but I'll add two more bigger questions. I think both of these are unrelated to this PR, but input variables in general, but I wanted to start here, and then I can create issues for them separately. Hidden input variables@stoph and I were taking a look at this and weren't sure when a customer would want to set a starting page other than "1" when setting up a loop block. I thought about this some more and can imagine situations where developers don't want to allow editorial users to adjust input variables beyond what's defined in code, e.g. when the API has a maximum number that can be returned. Do we think adding a Input variable names in the sidebar
We use the key as the label for the input, not the name defined in the config. Could we switch that to the name when it exists? |
| protected string $id; | ||
|
|
||
| final private function __construct( protected array $config ) { | ||
| $this->id = md5( wp_json_encode( [ get_called_class(), $config ] ) ); |
There was a problem hiding this comment.
One of the thoughts I had was to see if we could leverage the UUID from the config instead of doing it this way. But, I see that the problem is this isn't always available like for code based sources (Art Institute being an example of this).
Is there any benefit in changing that, so that all data source configs do have a UUID assigned to them? Then, we could use that UUID here and wherever else we might need to in the future?
There was a problem hiding this comment.
All good if the answer is no, or that it be punted to a future PR. This is just a thought that occurred to me when I tried to experiment with changing it to the UUID, and not seeing it reflect since the Art Institute didn't have one.
There was a problem hiding this comment.
Is there any benefit in changing that, so that all data source configs do have a UUID assigned to them? Then, we could use that UUID here and wherever else we might need to in the future?
It's a good question. We've discussed both sides. The major downsides of universally requiring UUIDs for all entities are:
- You have to delegate UUID creation to the user, and they can mess it up. We cannot create stable UUIDs on their behalf without persistence (which code-generated entities don't have). We can validate that a string looks like a UUID, but we can't really validate that it is unique or stable.
- There isn't really much benefit to the user that justifies making this a requirement. Code-generated UUIDs are not persisted and there is no central store, so there is no concept of "get by UUID." Even if there was, users have already created an instance of their entity and can freely pass it around in their code—which is probably easier and clearer than passing around a UUID string.
Another downside of UUIDs in this use case is that they have no functional value. They are just identifiers. The ID introduced in this PR, by contrast, is deterministic based on the inputs that influence its behavior. This is useful since we are using it to identify external attributes that can be applied to this entity (pagination variables). If the input changes, the ID changes, and it makes sense that the attributes might no longer apply. Meanwhile, a UUID might change for any number of reasons that have nothing to do with the entity's behavior.
There was a problem hiding this comment.
All that said, the main reason that this ID is useful at all is because block instances don't have stable identifiers. If they did, we would use them.
| 'per_page' => $per_page_variable, | ||
| ]; | ||
|
|
||
| if ( $cursor_variable ) { |
There was a problem hiding this comment.
Could this logic be documented just above this if statement, or as a function comment?
@smithjw1 I see the utility, but it feels like injecting the default value (done in your PR) also solves for this in a limited way. My instinct is that the issue you're describing is much more complex and speaks to a desire for developers to have more control over the selection UI—and the ensuing validation and user feedback. I don't think adding configuration toggles is a scalable approach for this; we need to provide some hooks for developers to inject their own UI.
Yes, it's a bit more invasive of a change and unrelated to pagination, so I'll tackle it in a separate PR and pull in your improvements as well. |
|
Added a filter for the variable name in 0acd772 |
This PR adds a pagination helper block that enables pagination on the front-end (render). It loosely follows the pattern of the Core query pagination block, but fully encapsulates the pagination links. This pagination block is automatically inserted whenever a collection that supports pagination is inserted.
The biggest change is moving logic from the
usePaginationVariableshook to PHP, so that both server-side and client-side code can reuse the logic. This logic is encapsulated in the newPaginationclass and consumed byQueryRunnerandBlockBindings.Pagination links are implemented via a single query variable (
rdb-pagination) that envelopes all possible pagination parameters (cursor, offset, page, etc.) and encodes them in base64. Example:?rdb-pagination=eyI3MDkzOWFmYWZmNDE4MWYwODI5MTUzMTk4Nzg1ODE0ZSI6eyJwYWdlIjozfX0.For now, we will only support navigating a single block's pagination thread at any time, but the implementation leaves room to support multiple threads in the future, should that surface as an important use case.
Automatic pagination block insertion
insert.mov
Front-end pagination
paginatino.mov