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

Simple pagination #1471

Merged
merged 11 commits into from
Sep 18, 2018
Merged

Simple pagination #1471

merged 11 commits into from
Sep 18, 2018

Conversation

mamhoff
Copy link
Contributor

@mamhoff mamhoff commented Sep 11, 2018

What is this pull request for?

This PR changes behaviour regarding the number of items per page in Alchemy admin controllers.
Prior to the PR, Alchemy would always return 25 items. If Alchemy-Devise was installed, that would set a session[:screen_size] value, the height of which was used to guess how many items fit on the screen.

For some of my clients (with a screen height of 768), we sometimes got as little as three items, with no way to override it.

Notable changes (remove if none)

  • Deprecates Alchemy::Admin::BaseController#per_page_value_for_screen_size
  • Introduces global configuration option items_per_page
  • Introduces Alchemy::Admin::BaseController#items_per_page
  • Allow using ?=per_page=25 to override pagination per value

Additional considerations

I thought about adding in a feature where users could choose the amount of items they prefer as a preference, however, I do not think that would work very well from a UI standpoint.

Besides, all the user logic is in Alchemy-Devise, so I actually think if we want that feature, we should add it there (and modify Alchemy's Base controller using some kind of extension mechanism).

Prior to this, Alchemy would guess based on the screen size of the user.

This deprecates the prior method which would rely on a session value
only set in `alchemy-devise`.
This logic also does not work any longer since the removal
of the screen-size-based pagination.
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks. This weird "feature" was flawed for a long time.

@tvdeyen
Copy link
Member

tvdeyen commented Sep 18, 2018

@mamhoff I added a per page select that is persisted in cookies.

alchemy test_admin_attachments_utf8 e2 9c 93 per_page 15 laptop with hidpi screen
alchemy test_admin_pictures_utf8 e2 9c 93 size large per_page 12 laptop with hidpi screen

@tvdeyen tvdeyen added this to the 4.1 milestone Sep 18, 2018
@tvdeyen tvdeyen merged commit 4c8c31a into AlchemyCMS:master Sep 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants