Views: expose items per page as config option

Created on 30 January 2025, 3 months ago

Problem/Motivation

I'm rendering a View and injecting arguments, but I also want to configure the amount of items to render.

Proposed resolution

  • Add extra optional number field to configuration of "eca_render_views" Action
  • Replace "views_embed_view" with the underlying implementation
  • If configuration, set the items per page
Feature request
Status

Active

Version

2.1

Component

Code

Created by

🇧🇪Belgium lammensj

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @lammensj
  • 🇩🇪Germany jurgenhaas Gottmadingen

    This sounds like an interesting improvement. I wonder, while being on it, if there are other properties too, that we may want to change at the same time.

  • 🇧🇪Belgium lammensj

    It depends a bit on how flexible you want it to be. If you search for "function set" on this page: https://api.drupal.org/api/drupal/core%21modules%21views%21src%21ViewExe..., you'll get a list of all the items you can alter/set before executing the View.

    For now, I'm just limiting it to the arguments (already there) and the items per page.

  • 🇧🇪Belgium lammensj

    However... Testing the change now... When I set the items per page on the View-object, it works when it's rendered initially. But the second page just loads the amount of items that is configured in the View itself...

  • Pipeline finished with Success
    3 months ago
    Total: 466s
    #410304
  • 🇩🇪Germany jurgenhaas Gottmadingen

    Left a couple of comments in the MR.

    When I set the items per page on the View-object, it works when it's rendered initially. But the second page just loads the amount of items that is configured in the View itself.

    That's strange. Maybe we could check in the views block plugin and check how they've done it there.

  • 🇧🇪Belgium lammensj

    I've implemented your remarks, thank you for the feedback.

    Regarding the items being returned for the second "page", I believe it's an issue coming from Views Infinite Scroll. I'll link the ticket. But this shouldn't stop this particular feature request.

  • Pipeline finished with Success
    3 months ago
    Total: 490s
    #414776
  • 🇧🇪Belgium lammensj

    For reference, this is how the Views Block plugin is overriding the items_per_page config item: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/views.... The ECA Action is doing the same thing.

  • Pipeline finished with Failed
    3 months ago
    Total: 353s
    #414835
  • Pipeline finished with Success
    3 months ago
    Total: 462s
    #414843
  • 🇩🇪Germany jurgenhaas Gottmadingen

    The MR looks great. We probably need a update hook so that existing models will be updated for the new config property.

    Having said that, the default value of the items_per_page should probably be an empty string, not NULL.

    Following your comment #7, does that mean that it works correctly if Views Infinite Scroll is disabled?

  • Pipeline finished with Failed
    24 days ago
    Total: 342s
    #460404
  • Pipeline finished with Failed
    24 days ago
    Total: 529s
    #460407
  • 🇧🇪Belgium lammensj

    Summary of the latest changes:
    - [eca_render]: Refactored the way the configuration key is translated to a value (either a token or a regular string)
    - [eca_render]: Added update-hook to update existing and their config
    - [eca_views]: Added eca_views_set_pagination_values-action to keep the pagination setting across different http-requests. The "issue" mentioned above, with views_infinite_scroll, was not related but did send me in the right direction. Overwriting the pagination via the eca_render_views-action works, but only for the initial rendering. When using ajax pagination, Views falls back to the setting of the View itself.
    So I had to tackle this from another angle: hooking into the pre-execute or pre-build event of that View. So I've created a "copy" of the eca_views_set_filter_value-action, and changed it so it sets the pagination- and offset-config.

    For my particular use-case (see screenshot) this approach now works: in one model, you render the View and alter it via the pre-build event. There's only one bug, not sure if it is one, and that's data of the event that is available. See separate issue.

  • Pipeline finished with Failed
    24 days ago
    Total: 463s
    #460790
  • Pipeline finished with Success
    24 days ago
    Total: 586s
    #460792
  • Status changed to Needs work 22 days ago
  • 🇩🇪Germany jurgenhaas Gottmadingen

    Left a few comments in the MR.

    While I like the new plugin ViewsSetPagination, I think this is a nice separate new feature, but it should be the "solution" for the issue with subsequent pager requests. The reason being it that this would only work if one has a complex ECA model always around which fixes the issue with the pager. That's not sustainable.

    We need to look into the rendered view as prepared by the render views plugin and see why this isn't working across multiple ajax pager requests. We need to get this to work without any extra events to hook into the logic. If core renders that view with a pager, it lets us page through without any issue, the same needs to be achievable by "custom" code, i.e. from within ECA.

  • Pipeline finished with Failed
    13 days ago
    Total: 483s
    #470104
  • Pipeline finished with Failed
    13 days ago
    Total: 532s
    #470264
  • 🇧🇪Belgium lammensj

    I incorporated the mentioned remarks.

    About the need for that second event: when I render the View with one of the default pagination options (mini or extensive), the same "issue" is visible: the next page contains the amount of items that was specified in the settings, and not how what it was actually rendered. This says to me that that setting is not passed between the consequential ajax calls.
    Only when disabling ajax, that overwritten amount of items is "retained". I put that between quotes because it's another render event that triggers the build of that View, with that separate value for the amount of items.

    So I'm assuming that, even without ECA, you would still have to use the pre-build hook to overwrite the amount of visible items. And some "initial" render code, but that's specific to this implementation. So yes, the change to the "Views"-action of the eca_render-module (specifying the amount of items) could be left out of this PR and replaced with a guideline: "if you want to change the amount of visible items per page, use the pre-build event to specify it".

  • 🇩🇪Germany jurgenhaas Gottmadingen

    Looks like the commit 1b39297b - Views: expose items per page as config option Active Allow exposed_input to be injected in the query sneaked in, it isn't really related to this issue, is it? Can you revert that? It should be a separate issue, but before creating one, have you tried if Views: Set filter value doesn't do that already?

    Other than that, I have a few more comments in the ViewsSetPagination class.

    With regard to the mis-behaviour of the pager when the pager settings get changed this way, I still believe this is either a core bug or the settings get injected too late, and we may have to use a different event so that it can be done earlier. Maybe we can xdebug that $pager->setItemsPerPage method to see when it's called by views itself, when it uses the regular setting that are not altered.

Production build 0.71.5 2024