From a7a2d37a6bf98b336955da9ab4616eb1483ee06d Mon Sep 17 00:00:00 2001
From: Cinder <cinder@e621.net>
Date: Thu, 6 Feb 2025 21:46:20 -0800
Subject: [PATCH] [Pagination] Fix overflow, usability, and accessibility
 issues (#898)

---
 app/helpers/pagination_helper.rb              | 67 ++++++++++++-------
 app/javascript/src/javascripts/paginator.js   |  2 +-
 .../src/styles/common/paginator.scss          | 15 +++--
 3 files changed, 55 insertions(+), 29 deletions(-)

diff --git a/app/helpers/pagination_helper.rb b/app/helpers/pagination_helper.rb
index 2e65d042f..d293d422a 100644
--- a/app/helpers/pagination_helper.rb
+++ b/app/helpers/pagination_helper.rb
@@ -2,22 +2,13 @@
 
 module PaginationHelper
   def sequential_paginator(records)
-    tag.div(class: "paginator") do
+    tag.nav(class: "pagination sequential", aria: { label: "Pagination" }) do
       return "" if records.try(:none?)
 
       html = "".html_safe
 
-      # Previous
-      html << link_to(records.is_first_page? ? "#" : nav_params_for("a#{records[0].id}"), class: "prev", id: "paginator-prev", rel: "prev", data: { shortcut: "a left", disabled: records.is_first_page? }) do
-        concat svg_icon(:chevron_left)
-        concat tag.span("Prev")
-      end
-
-      # Next
-      html << link_to(records.is_last_page? ? "#" : nav_params_for("b#{records[-1].id}"), class: "next", id: "paginator-next", rel: "next", data: { shortcut: "d right", disabled: records.is_last_page? }) do
-        concat tag.span("Next")
-        concat svg_icon(:chevron_right)
-      end
+      html << paginator_prev(nav_params_for("a#{records[0].id}"), disabled: records.is_first_page?)
+      html << paginator_next(nav_params_for("b#{records[-1].id}"), disabled: records.is_last_page?)
 
       html
     end
@@ -28,15 +19,11 @@ module PaginationHelper
       return sequential_paginator(records)
     end
 
-    tag.div(class: "paginator", data: { total: [records.total_pages, records.max_numbered_pages].min, current: records.current_page }) do
+    tag.nav(class: "pagination numbered", aria: { label: "Pagination" }, data: { total: [records.total_pages, records.max_numbered_pages].min, current: records.current_page }) do
       html = "".html_safe
 
       # Previous
-      has_prev = records.current_page < 2
-      html << link_to(has_prev ? "#" : nav_params_for(records.current_page - 1), class: "prev", id: "paginator-prev", rel: "prev", data: { shortcut: "a left", disabled: has_prev }) do
-        concat svg_icon(:chevron_left)
-        concat tag.span("Prev")
-      end
+      html << paginator_prev(nav_params_for(records.current_page - 1), disabled: records.current_page < 2)
 
       # Break
       html << tag.div(class: "break")
@@ -47,11 +34,7 @@ module PaginationHelper
       end
 
       # Next
-      has_next = records.current_page >= records.total_pages
-      html << link_to(has_next ? "#" : nav_params_for(records.current_page + 1), class: "next", id: "paginator-next", rel: "next", data: { shortcut: "d right", disabled: has_next }) do
-        concat tag.span("Next")
-        concat svg_icon(:chevron_right)
-      end
+      html << paginator_next(nav_params_for(records.current_page + 1), disabled: records.current_page >= records.total_pages)
 
       html
     end
@@ -59,6 +42,42 @@ module PaginationHelper
 
   private
 
+  def paginator_prev(link, disabled: false)
+    html = "".html_safe
+
+    if disabled
+      html << tag.span(class: "prev", id: "paginator-prev", data: { shortcut: "a left" }) do
+        concat svg_icon(:chevron_left)
+        concat tag.span("Prev")
+      end
+    else
+      html << link_to(link, class: "prev", id: "paginator-prev", rel: "prev", data: { shortcut: "a left" }) do
+        concat svg_icon(:chevron_left)
+        concat tag.span("Prev")
+      end
+    end
+
+    html
+  end
+
+  def paginator_next(link, disabled: false)
+    html = "".html_safe
+
+    if disabled
+      html << tag.span(class: "next", id: "paginator-next", data: { shortcut: "a left" }) do
+        concat tag.span("Next")
+        concat svg_icon(:chevron_right)
+      end
+    else
+      html << link_to(link, class: "next", id: "paginator-prev", rel: "next", data: { shortcut: "a left" }) do
+        concat tag.span("Next")
+        concat svg_icon(:chevron_right)
+      end
+    end
+
+    html
+  end
+
   def paginator_pages(records)
     small_window = 2
     large_window = 4
@@ -90,7 +109,7 @@ module PaginationHelper
     if page == 0
       html << link_to(svg_icon(:ellipsis), nav_params_for(0), class: "spacer")
     elsif page == records.current_page
-      html << tag.span(page, class: "page current")
+      html << tag.span(page, class: "page current", aria: { current: "page" })
     else
       html << link_to(page, nav_params_for(page), class: "page #{klass}")
     end
diff --git a/app/javascript/src/javascripts/paginator.js b/app/javascript/src/javascripts/paginator.js
index 434750a2c..1147d4203 100644
--- a/app/javascript/src/javascripts/paginator.js
+++ b/app/javascript/src/javascripts/paginator.js
@@ -12,7 +12,7 @@ Paginator.init_fasttravel = function (button) {
 };
 
 $(() => {
-  for (const one of $(".paginator a.spacer").get())
+  for (const one of $("nav.pagination a.spacer").get())
     Paginator.init_fasttravel($(one));
 });
 
diff --git a/app/javascript/src/styles/common/paginator.scss b/app/javascript/src/styles/common/paginator.scss
index 6ffd9a446..f00e19abe 100644
--- a/app/javascript/src/styles/common/paginator.scss
+++ b/app/javascript/src/styles/common/paginator.scss
@@ -1,4 +1,4 @@
-.paginator {
+nav.pagination {
   display: flex;
   flex-wrap: wrap;
 
@@ -8,6 +8,7 @@
 
   width: max-content;
   margin: 0 auto;
+  max-width: calc(100vw - 1rem);
 
   & > a, & > span {
     display: flex;
@@ -27,9 +28,8 @@
     }
   }
 
-  & > a[data-disabled="true"] {
-    color: var(--color-text);
-    pointer-events: none;
+  span.prev, span.next {
+    cursor: default;
   }
 
   // Ordering
@@ -99,3 +99,10 @@
     a.page.lg { display: flex; }
   }
 }
+
+
+// Sequential paginator
+.paginator.sequential {
+  gap: 5rem;
+  a span { display: block; }
+}