Add per_page option in Admin for participants#5480
Add per_page option in Admin for participants#5480agustibr wants to merge 8 commits intodecidim:masterfrom
Conversation
bd90b63 to
dd0dc8f
Compare
bd7e147 to
b551c1c
Compare
tramuntanal
left a comment
There was a problem hiding this comment.
thanks @agustibr , I'lve left just some comments regarding documentation and foundation.
decidim-admin/app/views/decidim/admin/officializations/index.html.erb
Outdated
Show resolved
Hide resolved
|
Thanks @tramuntanal I'll check your review |
b551c1c to
43ea266
Compare
…, consultations and initiatives
…atives and proposals
- proposals - assemblies - conferences - consultations - initiatives - participary processes
89966e2 to
67d6de8
Compare
|
@jarvisct I've removed all the functionality, so it only remains the functionality affecting participants |
| helper Decidim::Admin::IconLinkHelper | ||
| helper Decidim::Admin::MenuHelper | ||
| helper Decidim::Admin::ScopesHelper | ||
| helper Decidim::Admin::Paginable::PerPageHelper |
There was a problem hiding this comment.
Why not declaring this helper inside the Decidim::Admin::Paginable concern itself?
| module Paginable | ||
| # This module includes helpers the :per_page cell's option | ||
| module PerPageHelper | ||
| def per_page_options |
There was a problem hiding this comment.
If this method does not need to make use of Action View helpers, I would rather move this logic inside the Decidim::Admin::Paginable concern.
Instead, it would be nice to expose some helper method to easily render the cell from the views, i.e.:
| property :per_page, :per_page_range, :with_label | ||
|
|
||
| def path_for_num_per_page(num_per_page = per_page_range.first) | ||
| %(#{request.path}?per_page=#{num_per_page}) |
There was a problem hiding this comment.
For this to work alongside existing params it should be changed to something like:
controller.url_for(params.to_unsafe_h.merge(per_page: num_per_page))There was a problem hiding this comment.
Well this was intended, as when the :per_page value changes the :page value has to be cleared.
But yes maybe it could be removed only the :page and mantainthe rest of them.
🎩 What? Why?
Add pagination options to select how many results are shown per page in admin:
📌 Related Issues
📋 Subtasks
CHANGELOGentry📷 Screenshots (optional)