Skip to content

Commit

Permalink
WIP: Other improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
@ddnexus
ddnexus committed Feb 6, 2025
1 parent 73ed0fc commit f1e0567
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 49 deletions.
39 changes: 20 additions & 19 deletions CHANGELOG-10.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,25 +112,26 @@ The code is simpler and lighter, you can also override the lookup of dictionary

Your app likely uses just a little fraction of the renamed things in the list below, but we wrote them all for completeness:

| Type | Search | Replace | Notes |
|-------------|------------------|-----------------------------|---------------------------------------------------------------------------|
| Constructor | `pagy(` | `pagy_offset(` | Consistent with the other old and new constructors |
| Function | `Pagy.root` | `Pagy::ROOT` | Stop calling a method just to get a constant Pathname |
| Accessor | `pagy.vars` | `pagy.options` | They are actually options that don't change during execution |
| Exception | `VariableError` | `OptionError` | Error for the passed options. VariableError was not accurate |
| Accessor | `err.variable` | `err.option` | Accessor for OptionError instance variable, consistent with the class |
| Option | `size: 7` | `length: 7` | Series length: more accurate, and avoid confusion with other `size`s |
| Option | `ends: false` | `compact: true` | Compact-gapless series: the boolean inverse of `ends` |
| Option | `:page_param` | `:page_sym` | The '_param' could be confused with the actual param value |
| Option | `:limit_param` | `:limit_sym` | The '_param' could be confused with the actual param value |
| Method | `pagy_anchor` | `pagy_anchor_lambda` | It creates a lambda that creates an anchor tag, not the anchor tag itself |
| Method | `pagy_url_for` | `pagy_page_url` | The legacy name was causing rails-related expectations |
| Method/args | `label_for(page` | `label(page: page` | Simpler name: `page` is now a keywork argument |
| Method/args | `label(page` | `label(page: page` | Same name: `page` is now a keywork argument |
| Naming | `*prev*` | `*previous*` | Unabbreviated word everywhere (option, accessor, methods, CSS class) |
| Method | `pagy_prev_a` | `pagy_previous_anchor` | Unabbreviated words |
| Method | `pagy_next_a` | `pagy_next_anchor` | Unabbreviated words |

| Type | Search | Replace | Notes |
|-------------|------------------|------------------------|-----------------------------------------------------------------------------|
| Constructor | `pagy(` | `pagy_offset(` | Consistent with the other old and new constructors |
| Function | `Pagy.root` | `Pagy::ROOT` | Stop calling a method just to get a constant Pathname |
| Accessor | `pagy.vars` | `pagy.options` | They are actually options that don't change during execution |
| Exception | `VariableError` | `OptionError` | Error for the passed options. VariableError was not accurate |
| Accessor | `err.variable` | `err.option` | Accessor for OptionError instance variable, consistent with the class |
| Method | `pagy_anchor` | `pagy_anchor_lambda` | It creates a lambda that creates an anchor tag, not the anchor tag itself |
| Method | `pagy_url_for` | `pagy_page_url` | The legacy name was causing rails-related expectations |
| Method/args | `label_for(page` | `label(page: page` | Simpler name: `page` is now a keywork argument |
| Method/args | `label(page` | `label(page: page` | Same name: `page` is now a keywork argument |
| Method | `pagy_t` | `pagy_translate` | Unabbreviated word |
| Method | `pagy_prev_a` | `pagy_previous_anchor` | Unabbreviated words |
| Method | `pagy_next_a` | `pagy_next_anchor` | Unabbreviated words |
| Naming | `*prev*` | `*previous*` | Unabbreviated word everywhere (option, accessor, methods, CSS class) |
| Option | `size: 7` | `length: 7` | Series linear length: more accurate, and avoid confusion with other `size`s |
| Option | `ends: false` | `compact: true` | Compact-gapless series: the boolean inverse of `ends` |
| Option | `:page_param` | `:page_sym` | The '_param' could be confused with the actual param value |
| Option | `:limit_param` | `:limit_sym` | The '_param' could be confused with the actual param value |

#### Replace your `pagy.rb` config file

With no more `Pagy::DEFAULT` and no more extras to `require`, all the statements in your old version are obsolete, so it's better
Expand Down
2 changes: 0 additions & 2 deletions gem/lib/pagy.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ class Pagy

attr_reader :page, :count, :previous, :next, :in, :limit, :options, :last

alias pages last

# Define the hierarchical identity methods, overridden by the respective classes
def offset? = false
def countless? = false
Expand Down
4 changes: 4 additions & 0 deletions gem/lib/pagy/core/seriable.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ module Core
module Seriable
LENGTH = 7

def self.included(including)
including.alias_method :pages, :last
end

# Return the array of page numbers and :gap e.g. [1, :gap, 8, "9", 10, :gap, 36]
def series(length: @options[:length] || LENGTH, compact: @options[:compact], **)
raise OptionError.new(self, :length, 'to be an Integer >= 0', length) \
Expand Down
8 changes: 1 addition & 7 deletions gem/lib/pagy/frontend.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,8 @@
class Pagy
# Module to include in the app helper
module Frontend
module I18nGemOverride
def pagy_translate(...) = ::I18n.translate(...)
alias pagy_t pagy_translate
end

def self.translate_with_the_slower_i18n_gem!
prepend I18nGemOverride
prepend(Module.new { def pagy_translate(...) = ::I18n.translate(...) })
::I18n.load_path += Dir[ROOT.join('locales/*.yml')]
end

Expand All @@ -35,6 +30,5 @@ def pagy_anchor_lambda(pagy, anchor_string: nil, **)
def pagy_translate(key, **)
Pagy::I18n.translate(key, locale: (@pagy_locale ||= 'en'), **)
end
alias pagy_t pagy_translate
end
end
1 change: 0 additions & 1 deletion gem/lib/pagy/frontend/utils/nav_js.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ module NavJs
# Build the nav_js tag, with the specific tokens for the style
def tag(frontend, pagy, tokens, nav_classes, id: nil, aria_label: nil, **)
sequels = pagy.sequels(**)

%(<nav#{id && %( id="#{id}")} class="#{'pagy-rjs ' if sequels[0].size > 1}#{nav_classes}" #{
NavAriaLabel.attr(frontend, pagy, aria_label:)} #{
data = [:nj, tokens.values, sequels]
Expand Down
10 changes: 5 additions & 5 deletions test/pagy/frontend/i18n_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
app.test_i18n_call
end
it 'pluralizes' do
_(app.pagy_t('pagy.aria_label.previous')).must_equal "Previous"
_(app.pagy_t('pagy.item_name', count: 0)).must_equal 'items'
_(app.pagy_t('pagy.item_name', count: 1)).must_equal 'item'
_(app.pagy_t('pagy.item_name', count: 10)).must_equal 'items'
_(app.pagy_translate('pagy.aria_label.previous')).must_equal "Previous"
_(app.pagy_translate('pagy.item_name', count: 0)).must_equal 'items'
_(app.pagy_translate('pagy.item_name', count: 1)).must_equal 'item'
_(app.pagy_translate('pagy.item_name', count: 10)).must_equal 'items'
end
it 'handles missing paths' do
_(app.pagy_t('pagy.not_here')).must_equal 'Translation missing: en.pagy.not_here'
_(app.pagy_translate('pagy.not_here')).must_equal 'Translation missing: en.pagy.not_here'
end
end

Expand Down
30 changes: 15 additions & 15 deletions test/pagy/frontend_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,34 +19,34 @@

describe '#pagy_t' do
it 'pluralizes' do
_(app.pagy_t('pagy.aria_label.previous')).must_equal "Previous"
_(app.pagy_t('pagy.item_name', count: 0)).must_equal "items"
_(app.pagy_t('pagy.item_name', count: 1)).must_equal "item"
_(app.pagy_t('pagy.item_name', count: 10)).must_equal "items"
_(app.pagy_translate('pagy.aria_label.previous')).must_equal "Previous"
_(app.pagy_translate('pagy.item_name', count: 0)).must_equal "items"
_(app.pagy_translate('pagy.item_name', count: 1)).must_equal "item"
_(app.pagy_translate('pagy.item_name', count: 10)).must_equal "items"
end
it 'interpolates' do
_(app.pagy_t('pagy.info.no_items', count: 0)).must_rematch :info_0
_(app.pagy_t('pagy.info.single_page', count: 1)).must_rematch :info_1
_(app.pagy_t('pagy.info.single_page', count: 10)).must_rematch :info_10
_(app.pagy_t('pagy.info.multiple_pages', count: 10)).must_rematch :info_multi_10
_(app.pagy_t('pagy.info.multiple_pages', item_name: 'Products', count: 300, from: 101, to: 125)).must_rematch :info_multi_item_name
_(app.pagy_translate('pagy.info.no_items', count: 0)).must_rematch :info_0
_(app.pagy_translate('pagy.info.single_page', count: 1)).must_rematch :info_1
_(app.pagy_translate('pagy.info.single_page', count: 10)).must_rematch :info_10
_(app.pagy_translate('pagy.info.multiple_pages', count: 10)).must_rematch :info_multi_10
_(app.pagy_translate('pagy.info.multiple_pages', item_name: 'Products', count: 300, from: 101, to: 125)).must_rematch :info_multi_item_name
end
it 'handles missing keys' do
_(app.pagy_t('pagy.not_here')).must_equal '[translation missing: "pagy.not_here"]'
_(app.pagy_t('pagy.gap.not_here')).must_equal '[translation missing: "pagy.gap.not_here"]'
_(app.pagy_translate('pagy.not_here')).must_equal '[translation missing: "pagy.not_here"]'
_(app.pagy_translate('pagy.gap.not_here')).must_equal '[translation missing: "pagy.gap.not_here"]'
end
end

describe "Pagy::I18n" do
it 'switches :locale according to @pagy_locale' do
app.set_pagy_locale('nl')
_(app.pagy_t('pagy.item_name', count: 1)).must_equal "stuk"
_(app.pagy_translate('pagy.item_name', count: 1)).must_equal "stuk"
app.set_pagy_locale('en')
_(app.pagy_t('pagy.item_name', count: 1)).must_equal "item"
_(app.pagy_translate('pagy.item_name', count: 1)).must_equal "item"
app.set_pagy_locale('de')
_(app.pagy_t('pagy.item_name', count: 1)).must_equal "Eintrag"
_(app.pagy_translate('pagy.item_name', count: 1)).must_equal "Eintrag"
app.set_pagy_locale('unknown')
_ { app.pagy_t('pagy.item_name', count: 1) }.must_raise Errno::ENOENT
_ { app.pagy_translate('pagy.item_name', count: 1) }.must_raise Errno::ENOENT
app.instance_variable_set(:@pagy_locale, nil) # reset for other tests
end
end
Expand Down

0 comments on commit f1e0567

Please sign in to comment.