-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Update has_many.rb #2274
Update has_many.rb #2274
Conversation
disable paginate for has_many
Hi jubilee2, Thanks for the proposal. If my understanding of your code is correct, this change would allow something like this ?
|
Yes, It is correct! |
How about making it work with |
that would be a little more natural |
def paginate?
limit != false
end |
@jubilee2 - I think that's right. What do you think? Would you be up for modifying your PR? |
can you help me update the document? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the long delay. Let's see if we can get this merged!
lib/administrate/field/has_many.rb
Outdated
@@ -44,6 +44,10 @@ def selected_options | |||
def limit | |||
options.fetch(:limit, DEFAULT_LIMIT) | |||
end | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remember to remove this trailing space 🙂
Co-authored-by: Pablo Brasero <36066+pablobm@users.noreply.github.com>
Co-authored-by: Pablo Brasero <36066+pablobm@users.noreply.github.com>
resolved |
Thank you @ALL for this evolution. |
By the way, a bug was introduced here. It should have surfaced in CI but somehow it didn't 🤔 Oh well. I have created #2289 to fix this. Currently waiting for a fellow maintainer to review. |
not sure the CI not trigger on PR... take look into nonzero |
#2296 should fix issue |
disable paginate for has_many field
solve #2271