-
Notifications
You must be signed in to change notification settings - Fork 255
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
Enable new functionality for pagination information on grouped results #2140
Conversation
# grouped key specific i18n keys. `key` is the field being grouped | ||
def entry_name(options) | ||
begin | ||
I18n.t("blacklight.entry_name.grouped.#{key}", raise: true) |
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.
I think this can be collapsed using I18n fallbacks (and its pluralization methods?), something like:
I18n.t("blacklight.entry_name.grouped.#{key}", default: 'blacklight.entry_name.grouped.default', count: options[: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.
👍 I opted to leave the count out as that would require additional keys. Just using the rails helper for pluralization.
15720b2
to
dc4ff43
Compare
…to have pushed this into the model
a4fe97c
to
4e77f7a
Compare
options[:entry_name] | ||
elsif collection.respond_to? :model # DataMapper | ||
collection.model.model_name.human.downcase | ||
elsif collection.respond_to?(:model_name) && !collection.model_name.nil? # AR, Blacklight::PaginationMethods |
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.
This change is breaking our site which uses both solr and non solr documents. because collection.model_name.downcase throws an error when collection.model_name is nil.
else | ||
t('blacklight.entry_name.default') | ||
collection.entry_name(count: collection.size).downcase |
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.
Actually our issue is when collection.entry_name
returns nil we get an error. We pushed up a PR to fix this.
Enables projectblacklight/arclight#699 and replacing approach in projectblacklight/arclight#775.
A few things about this pull request. At first I aimed to target removal of
page_entries_info
in a Blacklight 8 release by removing our custom i18n keys and just using Kaminari's. This would have worked except for the two remaining Blacklight needs:We could accmmodate either one of those I think independently, but together I couldn't figure out a way to successfully push that to the
Blacklight::Solr::Response
in a sane way.I've hopefully simplified this method a bit so that future Blacklight maintainers have an easier time. 🤞