-
-
Notifications
You must be signed in to change notification settings - Fork 198
Upgrade Pagy #2423
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
base: master
Are you sure you want to change the base?
Upgrade Pagy #2423
Conversation
652201f to
2ee1552
Compare
| .col-auto | ||
| %p.mb-3 | ||
| != pagy_info(pagy, item_name: model.pluralize(pagy.count)) | ||
| != @pagy.info_tag(item_name: model.pluralize(@pagy.count)) |
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.
Maintenance note: we can pass in a local variable named pagy to this and other partials. That prepares us for Strict Locals in the future.
A nil/missin6 instance variable gives no meaningful error, whereas a missing local variable gives NameError - very clear and actionable.
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.
Seems like we’re already doing that, actually:
7 results - 4 files
app/views/admin/feedback/index.html.haml:
2 - if @feedback.any?
3: = render partial: 'shared/pagination', locals: { pagy: @pagy, model: 'feedback' }
4
26
27: = render partial: 'shared/pagination', locals: { pagy: @pagy, model: 'feedback' }
28
app/views/admin/members/events.html.haml:
17 %h3 Past RSVPs
18: = render partial: 'shared/pagination', locals: { pagy: @pagy, model: 'events' }
19
22
23: = render partial: 'shared/pagination', locals: { pagy: @pagy, model: 'events' }
app/views/admin/sponsors/index.html.haml:
15
16: = render partial: 'shared/pagination', locals: { pagy: @pagy, model: 'sponsor' }
17
43
44: = render partial: 'shared/pagination', locals: { pagy: @pagy, model: 'sponsor' }
app/views/coach/_coach.html.haml:
16
17: = render partial: 'shared/pagination', locals: { pagy: @pagy, model: 'coach' }
config/initializers/pagy.rb
Outdated
| # Pagy initializer file (6.2.0) | ||
| # Customize only what you really need and notice that the core Pagy works also without any of the following lines. | ||
| # Should you just cherry pick part of this file, please maintain the require-order of the extras | ||
| # Pagy initializer file (43.2.2) - UPGRADED from 6.2.0 |
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.
Ditch the history, it's super without it.
olleolleolle
left a comment
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.
Looks super! I only read the code changes, didn't look at the site in preview.
Seems like we are using it in select places only.
The new config experience is lovely.
|
I’ll update the rest either today or tomorrow — thanks @olleolleolle! |
Suggestion was “we can pass in a local variable named pagy to this and other partials. That prepares us for Strict Locals in the future.”
2ee1552 to
57e4b6f
Compare
Upgrade Pagy gem, following this upgrade guide: https://ddnexus.github.io/pagy/guides/upgrade-guide 🐸