-
-
Notifications
You must be signed in to change notification settings - Fork 255
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
feature: countless pagination #1998
Conversation
Code Climate has analyzed commit 606fe9b and detected 0 issues on this pull request. View more on Code Climate. |
We need translations for: records_selected_from_all_pages_html: All records selected from all pages
x_records_selected_from_page_html: <span class="font-bold text-gray-700">%{selected}</span> records selected on this page
|
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.
Let's change the API from slef.countles
to
self.pagination = {
type: :normal/:countless/:countless_minimal
}
This will allow us to future-proof the pagination API.
About the The result on the image can be obtained by
|
spec/system/avo/select_all_spec.rb
Outdated
context "select first page" do | ||
it "releases the fish from the selected page" do | ||
visit url | ||
# describe "without applyed filters" do |
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.
Why did we comment these tests?
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.
My bad, I commeted on dev when working on it and forgot to uncomment 🤦🏼
end | ||
|
||
@pagy, @models = pagy(@query, items: @index_params[:per_page], link_extra: "data-turbo-frame=\"#{params[:turbo_frame]}\"", size: [1, 2, 2, 1], params: extra_pagy_params) | ||
@pagy, @models = @resource.apply_pagination(index_params: @index_params, query: @query) |
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.
Let's extract this into it's own apply_pagination
method so users can override it more easily.
This can be a great pattern going forward.
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.
Left a few comments. Good job!
Let's use |
This PR has been merged into Please check the release guide for more information. |
Description
Fixes #1748
Added new DSL to resource:
Checklist:
Manual review steps
self.pagination = -> do { type: :countless, size: [] } end
to any resourceManual reviewer: please leave a comment with output from the test if that's the case.