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.

Sign up for GitHub

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

Jump to bottom

[WIP] Temporarily fixes double pagination on /questions/tag/ #8479

Closed
wants to merge 1 commit into from
Closed

[WIP] Temporarily fixes double pagination on /questions/tag/ #8479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/views/questions/_questions.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
<% end %>
<% end %>
</div>

<div class="text-center mt-3">
<% if @pagy %>
<%== pagy_bootstrap_nav @pagy %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/tag/show/_tab_content.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
</div>

<div class="tab-pane" id="answered-tab">
<% if @answered_questions %>
<% if @answered_questions.present? %>
<%= render partial: "users/answered" %>
<% end %>
</div>
Expand Down
14 changes: 8 additions & 6 deletions app/views/users/_answered.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,12 @@
</div>
</div>

<div class="text-center">
<% if @pagy %>
<%== pagy_bootstrap_nav @pagy %>
<% else %>
<%= will_paginate @questions, :renderer => WillPaginate::ActionView::BootstrapLinkRenderer unless @unpaginated || @questions.empty? %>
<% if @answered_questions.present? %>
Copy link
Member

Choose a reason for hiding this comment

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

So, is the issue here that one of the pagination elements is for answered questions and the other is for unanswered? Looking at the two includes at

plots2/app/views/tag/show/_tab_content.html.erb

Lines 28 to 37 in 0633521

<div class="tab-content">
<div class="tab-pane active" id="asked-tab">
<%= render partial: "questions/new_question" %>
</div>
<div class="tab-pane" id="answered-tab">
<% if @answered_questions %>
<%= render partial: "users/answered" %>
<% end %>
</div>

To test this theory, we could try commenting out one and seeing if the double pagination helper disappears.

Just to note, I believe we are OK eliminating the distinction between answered and unanswered in this display. Pinging @ebarry to confirm though.

Copy link
Member

Choose a reason for hiding this comment

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

yes it does sound ok to eliminate the "answered | unanswered" toggle and the associated pagination.

One idea towards helping people choose which question card to potentially click on is to pull out the # of comments that is contained in the ... on each question card, and perhaps show these #s directly on the card to give people a sense of where there's a lot of dialogue/engagement.

Copy link
Member

Choose a reason for hiding this comment

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

OK, great, so confirmed we can just remove the toggle, and hopefully that resolves the double pagination too!

I'm breaking out the design discussion portion into its own issue at: #8731

Copy link
Member

Choose a reason for hiding this comment

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

Also we will move over to #8689 -- thanks!

<div class="text-center">
<% if @pagy %>
<%== pagy_bootstrap_nav @pagy %>
<% else %>
<%= will_paginate @answered_questions, :renderer => WillPaginate::ActionView::BootstrapLinkRenderer unless @unpaginated || @answered_questions.empty? %>
<% end %>
</div>
<% end %>
</div>