-
Notifications
You must be signed in to change notification settings - Fork 61
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
[ENHANCEMENT] Proposed fix to make row pagination count a param instead of hardcoded 5 #277
Comments
Pretty good idea! Amusingly, it looks like I don't actually use
As to how you access it, you would want to do it similar to this (it's important to set a default because folks might not have this set, and we don't want to break existing configs):
so the code would probably look like this (I haven't tested, so it might not work):
I would slightly recommend rather than creating a new config value, using the existing config. I think that you can get to it with Giving this a little more thought...the best move might be to just change it to:
That will preserve the default behavior of 5 rows in a Row layout, but allow the use of the standard If you want to submit a PR with this change, I'd be happy to take a look! |
Thanks Matt. I tried your code snippet but it fails at accessing the top-level
(Strangely, I am now curious how the Thoughts? |
I think that the I'll experiment with this myself later - not sure if I can get to it today, but in the next day or so! |
Reference:
|
I think the following fix (tested and it works) might be best: just using the built-in
For people using row layout, pagination will change from 5 to Behavior a) is unavoidable even if we add logic to default to 5 when I do not think it is worth being backward compatible to behavior b), as I suspect few users have no We can address this by noting this change/functionality in the release notes. |
I agree! If you want to submit a PR, I'll cut a release for it (and you're right, if we just put this in the release notes for this version, that will handle the change, for those folks who read release notes!) |
Thanks Matt. I'll submit a PR by Saturday. |
Have submitted PR #278 for your review and action at your convenience. Thanks again. Cheers. |
When
site_layout = "row"
, pagination is currently hardcoded at 5 rows per page. Here's a fix for making the row count a param,row_paginate
, instead:Instead of making a pull request, I thought I'd ask for comment here first because there already exists a top level param called
paginate
used forsite_layout = "grid"
, and it might be better to just use that param for the"row"
case as well. I am happy doing that, though I don't know how to access that param (what's the variable name to use?.Site.paginate
does not work).Matt, happy to make a pull request based on your input. Thanks.
The text was updated successfully, but these errors were encountered: