Skip to content
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

Merged
merged 15 commits into from
Nov 16, 2023
Merged

feature: countless pagination #1998

merged 15 commits into from
Nov 16, 2023

Conversation

Paul-Bob
Copy link
Contributor

@Paul-Bob Paul-Bob commented Oct 27, 2023

Description

Fixes #1748

Added new DSL to resource:

self.pagination = -> do
  {
    type: :countless, # default -> :normal
    size: []          # default -> [1, 2, 2, 1]
  }
end
class CourseLinkResource < Avo::BaseResource
  ...
  self.pagination = -> do
    {
      type: :countless,
      size: []
    }
  end

  # OR

  self.pagination = {
    type: :countless,
    size: []
  }
  ...
end

image

image

image

image

Checklist:

Manual review steps

  1. Add self.pagination = -> do { type: :countless, size: [] } end to any resource
  2. Verify resource's index page

Manual reviewer: please leave a comment with output from the test if that's the case.

@Paul-Bob Paul-Bob self-assigned this Oct 27, 2023
@codeclimate
Copy link

codeclimate bot commented Oct 27, 2023

Code Climate has analyzed commit 606fe9b and detected 0 issues on this pull request.

View more on Code Climate.

@Paul-Bob
Copy link
Contributor Author

Paul-Bob commented Oct 27, 2023

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
  • avo.ar.yml
  • avo.fr.yml
  • avo.nb.yml
  • avo.nn.yml
  • avo.tr.yml

Copy link
Collaborator

@adrianthedev adrianthedev left a 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.

@Paul-Bob
Copy link
Contributor Author

Paul-Bob commented Nov 6, 2023

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 countless_minimal we talked that we want to achieve this:

image

The result on the image can be obtained by countless and size: [] so I added the size key on the pagination hash, keeping our default value. Each user can customize the size or not... The documentation explains it well.

countless_minimal will actually hide the whole pages navigation. Do we really need to implement it? I think (not sure) we can implement it on a next PR with a custom navigator component where we implement our "Prev" and "Next" buttons without using the pagy helpers. Pagy helpers can´t be used with countless_minimal

image

@Paul-Bob Paul-Bob closed this Nov 6, 2023
@Paul-Bob Paul-Bob reopened this Nov 6, 2023
context "select first page" do
it "releases the fish from the selected page" do
visit url
# describe "without applyed filters" do
Copy link
Collaborator

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?

Copy link
Contributor Author

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)
Copy link
Collaborator

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.

Copy link
Collaborator

@adrianthedev adrianthedev left a 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!

@adrianthedev
Copy link
Collaborator

Let's use deafult instead of nornmal

@Paul-Bob Paul-Bob merged commit b216d3e into main Nov 16, 2023
11 of 13 checks passed
@Paul-Bob Paul-Bob deleted the feature/countless_pagination branch November 16, 2023 08:19
Copy link
Contributor

This PR has been merged into main. The functionality will be available in the next release.

Please check the release guide for more information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Let "total count" be hidden from pagination for large tables.
2 participants