From c33931b613c7da4cc2c22ff8411c38556dc579cb Mon Sep 17 00:00:00 2001 From: Ian McCowan Date: Sun, 25 Feb 2018 18:31:28 -0800 Subject: [PATCH] Fix prev/next links on public profile page (#6497) * Fix prev/next links on public profile page * Don't make pagination urls if no available statuses * Fix empty check method * Put left chevron before prev page link * Add scope for pagination "starting at" a given id * Status pagination try 2: s/prev/older and s/next/newer "older" on left, "newer" on right Use new scope for "newer" link Extract magic 20 page size to constant Remove max_id from feed pagination as it's not respected * Reinstate max_id for accounts atom stream * normalize --- app/controllers/accounts_controller.rb | 36 ++++++++++++++++---- app/javascript/styles/mastodon/accounts.scss | 16 ++++----- app/models/concerns/paginable.rb | 9 +++++ app/views/accounts/show.html.haml | 7 ++-- config/locales/en.yml | 2 ++ 5 files changed, 53 insertions(+), 17 deletions(-) diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 69fd20e27..7bf35825f 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class AccountsController < ApplicationController + PAGE_SIZE = 20 + include AccountControllerConcern before_action :set_cache_headers @@ -16,13 +18,16 @@ class AccountsController < ApplicationController end @pinned_statuses = cache_collection(@account.pinned_statuses, Status) if show_pinned_statuses? - @statuses = filtered_statuses.paginate_by_max_id(20, params[:max_id], params[:since_id]) + @statuses = filtered_status_page(params) @statuses = cache_collection(@statuses, Status) - @next_url = next_url unless @statuses.empty? + unless @statuses.empty? + @older_url = older_url if @statuses.last.id > filtered_statuses.last.id + @newer_url = newer_url if @statuses.first.id < filtered_statuses.first.id + end end format.atom do - @entries = @account.stream_entries.where(hidden: false).with_includes.paginate_by_max_id(20, params[:max_id], params[:since_id]) + @entries = @account.stream_entries.where(hidden: false).with_includes.paginate_by_max_id(PAGE_SIZE, params[:max_id], params[:since_id]) render xml: OStatus::AtomSerializer.render(OStatus::AtomSerializer.new.feed(@account, @entries.reject { |entry| entry.status.nil? })) end @@ -69,13 +74,22 @@ class AccountsController < ApplicationController @account = Account.find_local!(params[:username]) end - def next_url + def older_url + ::Rails.logger.info("older: max_id #{@statuses.last.id}, url #{pagination_url(max_id: @statuses.last.id)}") + pagination_url(max_id: @statuses.last.id) + end + + def newer_url + pagination_url(min_id: @statuses.first.id) + end + + def pagination_url(max_id: nil, min_id: nil) if media_requested? - short_account_media_url(@account, max_id: @statuses.last.id) + short_account_media_url(@account, max_id: max_id, min_id: min_id) elsif replies_requested? - short_account_with_replies_url(@account, max_id: @statuses.last.id) + short_account_with_replies_url(@account, max_id: max_id, min_id: min_id) else - short_account_url(@account, max_id: @statuses.last.id) + short_account_url(@account, max_id: max_id, min_id: min_id) end end @@ -86,4 +100,12 @@ class AccountsController < ApplicationController def replies_requested? request.path.ends_with?('/with_replies') end + + def filtered_status_page(params) + if params[:min_id].present? + filtered_statuses.paginate_by_min_id(PAGE_SIZE, params[:min_id]).reverse + else + filtered_statuses.paginate_by_max_id(PAGE_SIZE, params[:max_id], params[:since_id]).to_a + end + end end diff --git a/app/javascript/styles/mastodon/accounts.scss b/app/javascript/styles/mastodon/accounts.scss index 9015d04cb..c812766a1 100644 --- a/app/javascript/styles/mastodon/accounts.scss +++ b/app/javascript/styles/mastodon/accounts.scss @@ -233,8 +233,8 @@ a, .current, - .next, - .prev, + .newer, + .older, .page, .gap { font-size: 14px; @@ -257,13 +257,13 @@ cursor: default; } - .prev, - .next { + .older, + .newer { text-transform: uppercase; color: $ui-secondary-color; } - .prev { + .older { float: left; padding-left: 0; @@ -273,7 +273,7 @@ } } - .next { + .newer { float: right; padding-right: 0; @@ -295,8 +295,8 @@ display: none; } - .next, - .prev { + .newer, + .older { display: inline-block; } } diff --git a/app/models/concerns/paginable.rb b/app/models/concerns/paginable.rb index 6061bf9bd..66695677e 100644 --- a/app/models/concerns/paginable.rb +++ b/app/models/concerns/paginable.rb @@ -10,5 +10,14 @@ module Paginable query = query.where(arel_table[:id].gt(since_id)) if since_id.present? query } + + # Differs from :paginate_by_max_id in that it gives the results immediately following min_id, + # whereas since_id gives the items with largest id, but with since_id as a cutoff. + # Results will be in ascending order by id. + scope :paginate_by_min_id, ->(limit, min_id = nil) { + query = reorder(arel_table[:id]).limit(limit) + query = query.where(arel_table[:id].gt(min_id)) if min_id.present? + query + } end end diff --git a/app/views/accounts/show.html.haml b/app/views/accounts/show.html.haml index accad5f78..21c585dab 100644 --- a/app/views/accounts/show.html.haml +++ b/app/views/accounts/show.html.haml @@ -39,6 +39,9 @@ = render partial: 'stream_entries/status', collection: @statuses, as: :status - - if @statuses.size == 20 + - if @newer_url || @older_url .pagination - = link_to safe_join([t('pagination.next'), fa_icon('chevron-right')], ' '), @next_url, class: 'next', rel: 'next' + - if @older_url + = link_to safe_join([fa_icon('chevron-left'), t('pagination.older')], ' '), @older_url, class: 'older', rel: 'older' + - if @newer_url + = link_to safe_join([t('pagination.newer'), fa_icon('chevron-right')], ' '), @newer_url, class: 'newer', rel: 'newer' diff --git a/config/locales/en.yml b/config/locales/en.yml index 071c41290..026426c84 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -545,7 +545,9 @@ en: trillion: T unit: '' pagination: + newer: Newer next: Next + older: Older prev: Prev truncate: "…" preferences: