From aa0feab91118afa71204cd8e8def0b01bd4a9ece Mon Sep 17 00:00:00 2001 From: "Adam C. Stephens" Date: Sat, 22 Aug 2026 08:56:10 -0400 Subject: [PATCH] web: landed the viewport on the list when changing pages Refs: TRK-429 Assisted-By: Claude Opus 5 --- assets/js/app.js | 15 +++ lib/tracker_web/components/pagination.ex | 58 ++++++------ lib/tracker_web/live/change_live/index.ex | 31 +------ lib/tracker_web/live/change_live/show.ex | 29 +----- lib/tracker_web/live/channel_live/show.ex | 1 + lib/tracker_web/live/inbox_live/index.ex | 23 ++--- lib/tracker_web/live/maintainer_live/index.ex | 19 +--- lib/tracker_web/live/maintainer_live/show.ex | 29 +----- lib/tracker_web/live/option_live/show.ex | 1 + lib/tracker_web/live/package_live/index.ex | 17 +--- lib/tracker_web/live/package_live/show.ex | 31 +------ lib/tracker_web/live/team_live/index.ex | 15 +-- lib/tracker_web/live/team_live/show.ex | 29 +----- .../components/pagination_test.exs | 93 +++++++------------ .../live/package_live/index_test.exs | 2 +- 15 files changed, 97 insertions(+), 296 deletions(-) diff --git a/assets/js/app.js b/assets/js/app.js index b594a36..c4c0078 100644 --- a/assets/js/app.js +++ b/assets/js/app.js @@ -37,6 +37,21 @@ Hooks.UpdateURL = { } } +Hooks.PageAnchor = { + mounted() { + this.page = this.el.dataset.page + }, + updated() { + if (this.el.dataset.page === this.page) return + this.page = this.el.dataset.page + let list = document.getElementById(this.el.dataset.anchor) + if (!list) return + // A patch keeps the viewport where it was, which strands mobile readers at + // the end of the new page; the no-JS path gets this from the URL fragment. + requestAnimationFrame(() => list.scrollIntoView({block: "start"})) + } +} + Hooks.AnchorExpand = { mounted() { this.expandAfterRender() diff --git a/lib/tracker_web/components/pagination.ex b/lib/tracker_web/components/pagination.ex index e94ab6b..8992b1f 100644 --- a/lib/tracker_web/components/pagination.ex +++ b/lib/tracker_web/components/pagination.ex @@ -2,15 +2,13 @@ defmodule TrackerWeb.Pagination do @moduledoc """ Pagination controls for paged lists. - Emits `prev-page` and `next-page` events for the parent LiveView to handle, - or renders links when `prev_path`/`next_path` are given. + Renders prev/next links to `prev_path`/`next_path`, anchored at the list + they page through. """ use TrackerWeb, :html @doc """ - Renders pagination controls with prev/next buttons and page indicator. - - Emits `prev-page` and `next-page` events for the parent LiveView to handle. + Renders pagination controls with prev/next links and a page indicator. ## Examples @@ -19,6 +17,9 @@ defmodule TrackerWeb.Pagination do current_page={@current_page} has_prev_page?={@has_prev_page?} has_next_page?={@has_next_page?} + prev_path={~p"/packages?page=1"} + next_path={~p"/packages?page=3"} + anchor="packages" /> """ attr :total_pages, :integer, @@ -28,24 +29,31 @@ defmodule TrackerWeb.Pagination do attr :current_page, :integer, required: true attr :has_prev_page?, :boolean, default: false attr :has_next_page?, :boolean, default: false + attr :prev_path, :string, required: true, doc: "URL for the previous page" + attr :next_path, :string, required: true, doc: "URL for the next page" - attr :prev_path, :string, - default: nil, + attr :anchor, :string, + required: true, doc: - "URL for the previous page (no-JS fallback). When set, renders an instead of a button." - - attr :next_path, :string, - default: nil, - doc: "URL for the next page (no-JS fallback). When set, renders an instead of a button." + "DOM id of the list being paged. The fragment lands a full page load at the top of the list; the hook does the same for a LiveView patch." def controls(assigns) do + assigns = + assigns + |> assign(:prev_path, anchored(assigns.prev_path, assigns.anchor)) + |> assign(:next_path, anchored(assigns.next_path, assigns.anchor)) + ~H""" """ end + defp anchored(path, anchor), do: path <> "#" <> anchor + defp show_pagination?(nil, has_prev_page?, has_next_page?), do: has_prev_page? or has_next_page? defp show_pagination?(total_pages, _has_prev_page?, _has_next_page?), do: total_pages > 1 end diff --git a/lib/tracker_web/live/change_live/index.ex b/lib/tracker_web/live/change_live/index.ex index 5699c95..dde182d 100644 --- a/lib/tracker_web/live/change_live/index.ex +++ b/lib/tracker_web/live/change_live/index.ex @@ -63,6 +63,7 @@ defmodule TrackerWeb.ChangeLive.Index do filter_extras(@base_ref_filter, @in_channel_filter?) ) } + anchor="changes" /> """ end @@ -134,36 +135,6 @@ defmodule TrackerWeb.ChangeLive.Index do {:noreply, socket} end - @impl true - def handle_event("next-page", _params, socket) do - tp = socket.assigns.table_params - - {:noreply, - push_patch(socket, - to: - TableParams.to_path( - %{tp | page: tp.page + 1}, - "/changes", - filter_extras(socket.assigns.base_ref_filter, socket.assigns.in_channel_filter?) - ) - )} - end - - @impl true - def handle_event("prev-page", _params, socket) do - tp = socket.assigns.table_params - - {:noreply, - push_patch(socket, - to: - TableParams.to_path( - %{tp | page: max(tp.page - 1, 1)}, - "/changes", - filter_extras(socket.assigns.base_ref_filter, socket.assigns.in_channel_filter?) - ) - )} - end - defp load_changes(socket) do tp = socket.assigns.table_params channel_name = TrackerWeb.Lens.channel_name(socket.assigns.lens) diff --git a/lib/tracker_web/live/change_live/show.ex b/lib/tracker_web/live/change_live/show.ex index 658d82c..98393e2 100644 --- a/lib/tracker_web/live/change_live/show.ex +++ b/lib/tracker_web/live/change_live/show.ex @@ -222,6 +222,7 @@ defmodule TrackerWeb.ChangeLive.Show do "/changes/#{@change.number}" ) } + anchor="affected-packages" /> <% end %> @@ -495,34 +496,6 @@ defmodule TrackerWeb.ChangeLive.Show do {:noreply, socket} end - @impl true - def handle_event("next-page", _params, socket) do - tp = socket.assigns.table_params - - {:noreply, - push_patch(socket, - to: - TableParams.to_path( - %{tp | page: tp.page + 1}, - "/changes/#{socket.assigns.change.number}" - ) - )} - end - - @impl true - def handle_event("prev-page", _params, socket) do - tp = socket.assigns.table_params - - {:noreply, - push_patch(socket, - to: - TableParams.to_path( - %{tp | page: max(tp.page - 1, 1)}, - "/changes/#{socket.assigns.change.number}" - ) - )} - end - defp load_packages(socket, change_id) do tp = socket.assigns.table_params package_count = socket.assigns.change.package_count || 0 diff --git a/lib/tracker_web/live/channel_live/show.ex b/lib/tracker_web/live/channel_live/show.ex index 787f46e..11d8499 100644 --- a/lib/tracker_web/live/channel_live/show.ex +++ b/lib/tracker_web/live/channel_live/show.ex @@ -66,6 +66,7 @@ defmodule TrackerWeb.ChannelLive.Show do has_next_page?={@has_next_page?} prev_path={TableParams.page_path(@table_params, @current_page - 1, "/channels/#{@channel}")} next_path={TableParams.page_path(@table_params, @current_page + 1, "/channels/#{@channel}")} + anchor="revisions" /> diff --git a/lib/tracker_web/live/inbox_live/index.ex b/lib/tracker_web/live/inbox_live/index.ex index 348beba..3e0d240 100644 --- a/lib/tracker_web/live/inbox_live/index.ex +++ b/lib/tracker_web/live/inbox_live/index.ex @@ -114,14 +114,6 @@ defmodule TrackerWeb.InboxLive.Index do |> reset_to_first_page() end - def handle_event("next-page", _params, socket) do - {:noreply, patch_to_page(socket, socket.assigns.table_params.page + 1)} - end - - def handle_event("prev-page", _params, socket) do - {:noreply, patch_to_page(socket, max(socket.assigns.table_params.page - 1, 1))} - end - def handle_event("mark-all-read", _params, socket) do user = socket.assigns.current_user @@ -332,12 +324,14 @@ defmodule TrackerWeb.InboxLive.Index do Nothing matches these filters. -
- - - <.row :for={n <- rows} n={n} now={@now} version_changes={@version_changes} /> - -
+
+
+ + + <.row :for={n <- rows} n={n} now={@now} version_changes={@version_changes} /> + +
+
""" diff --git a/lib/tracker_web/live/maintainer_live/index.ex b/lib/tracker_web/live/maintainer_live/index.ex index 34ed952..66a997e 100644 --- a/lib/tracker_web/live/maintainer_live/index.ex +++ b/lib/tracker_web/live/maintainer_live/index.ex @@ -28,6 +28,7 @@ defmodule TrackerWeb.MaintainerLive.Index do has_next_page?={@has_next_page?} prev_path={TableParams.page_path(@table_params, @current_page - 1, "/maintainers")} next_path={TableParams.page_path(@table_params, @current_page + 1, "/maintainers")} + anchor="maintainers" /> """ end @@ -76,24 +77,6 @@ defmodule TrackerWeb.MaintainerLive.Index do {:noreply, socket} end - @impl true - def handle_event("next-page", _params, socket) do - tp = socket.assigns.table_params - - {:noreply, - push_patch(socket, to: TableParams.to_path(%{tp | page: tp.page + 1}, "/maintainers"))} - end - - @impl true - def handle_event("prev-page", _params, socket) do - tp = socket.assigns.table_params - - {:noreply, - push_patch(socket, - to: TableParams.to_path(%{tp | page: max(tp.page - 1, 1)}, "/maintainers") - )} - end - defp load_maintainers(socket) do tp = socket.assigns.table_params diff --git a/lib/tracker_web/live/maintainer_live/show.ex b/lib/tracker_web/live/maintainer_live/show.ex index 86ae841..74039c6 100644 --- a/lib/tracker_web/live/maintainer_live/show.ex +++ b/lib/tracker_web/live/maintainer_live/show.ex @@ -101,6 +101,7 @@ defmodule TrackerWeb.MaintainerLive.Show do "/maintainers/#{@maintainer.github}" ) } + anchor="maintainer-packages" /> """ end @@ -148,34 +149,6 @@ defmodule TrackerWeb.MaintainerLive.Show do )} end - @impl true - def handle_event("next-page", _params, socket) do - tp = socket.assigns.table_params - - {:noreply, - push_patch(socket, - to: - TableParams.to_path( - %{tp | page: tp.page + 1}, - "/maintainers/#{socket.assigns.maintainer.github}" - ) - )} - end - - @impl true - def handle_event("prev-page", _params, socket) do - tp = socket.assigns.table_params - - {:noreply, - push_patch(socket, - to: - TableParams.to_path( - %{tp | page: max(tp.page - 1, 1)}, - "/maintainers/#{socket.assigns.maintainer.github}" - ) - )} - end - defp reload_page_data(socket) do maintainer = socket.assigns.maintainer tp = socket.assigns.table_params diff --git a/lib/tracker_web/live/option_live/show.ex b/lib/tracker_web/live/option_live/show.ex index abe0ff0..7a31e97 100644 --- a/lib/tracker_web/live/option_live/show.ex +++ b/lib/tracker_web/live/option_live/show.ex @@ -106,6 +106,7 @@ defmodule TrackerWeb.OptionLive.Show do has_next_page?={@has_next_page?} prev_path={TableParams.page_path(@table_params, @current_page - 1, show_path(@prefix))} next_path={TableParams.page_path(@table_params, @current_page + 1, show_path(@prefix))} + anchor="matching-options" /> diff --git a/lib/tracker_web/live/package_live/index.ex b/lib/tracker_web/live/package_live/index.ex index 91a4c1e..7a4c5aa 100644 --- a/lib/tracker_web/live/package_live/index.ex +++ b/lib/tracker_web/live/package_live/index.ex @@ -29,6 +29,7 @@ defmodule TrackerWeb.PackageLive.Index do has_next_page?={@has_next_page?} prev_path={TableParams.page_path(@table_params, @current_page - 1, "/packages")} next_path={TableParams.page_path(@table_params, @current_page + 1, "/packages")} + anchor="packages" /> """ end @@ -75,22 +76,6 @@ defmodule TrackerWeb.PackageLive.Index do {:noreply, socket} end - @impl true - def handle_event("next-page", _params, socket) do - tp = socket.assigns.table_params - - {:noreply, - push_patch(socket, to: TableParams.to_path(%{tp | page: tp.page + 1}, "/packages"))} - end - - @impl true - def handle_event("prev-page", _params, socket) do - tp = socket.assigns.table_params - - {:noreply, - push_patch(socket, to: TableParams.to_path(%{tp | page: max(tp.page - 1, 1)}, "/packages"))} - end - defp load_packages(socket) do tp = socket.assigns.table_params channel_id = TrackerWeb.Lens.channel_id(socket.assigns.lens) diff --git a/lib/tracker_web/live/package_live/show.ex b/lib/tracker_web/live/package_live/show.ex index 246f231..306fec1 100644 --- a/lib/tracker_web/live/package_live/show.ex +++ b/lib/tracker_web/live/package_live/show.ex @@ -253,6 +253,7 @@ defmodule TrackerWeb.PackageLive.Show do %{version: @version_filter, all_revisions: @all_revisions?} ) } + anchor="revisions" />

@@ -637,36 +638,6 @@ defmodule TrackerWeb.PackageLive.Show do )} end - @impl true - def handle_event("next-page", _params, socket) do - tp = socket.assigns.table_params - - {:noreply, - push_patch(socket, - to: - revisions_path( - socket.assigns.package.attribute, - %{tp | page: tp.page + 1}, - extra_params(socket) - ) - )} - end - - @impl true - def handle_event("prev-page", _params, socket) do - tp = socket.assigns.table_params - - {:noreply, - push_patch(socket, - to: - revisions_path( - socket.assigns.package.attribute, - %{tp | page: max(tp.page - 1, 1)}, - extra_params(socket) - ) - )} - end - defp load_revisions(package_id, channel_id, version_filter, offset, page_size) do Tracker.Nixpkgs.PackageHistory.revisions_by_package(package_id, channel_id, version: version_filter, diff --git a/lib/tracker_web/live/team_live/index.ex b/lib/tracker_web/live/team_live/index.ex index 41a38c9..90c7905 100644 --- a/lib/tracker_web/live/team_live/index.ex +++ b/lib/tracker_web/live/team_live/index.ex @@ -29,6 +29,7 @@ defmodule TrackerWeb.TeamLive.Index do has_next_page?={@has_next_page?} prev_path={TableParams.page_path(@table_params, @current_page - 1, "/teams")} next_path={TableParams.page_path(@table_params, @current_page + 1, "/teams")} + anchor="teams" /> """ end @@ -70,20 +71,6 @@ defmodule TrackerWeb.TeamLive.Index do {:noreply, socket} end - @impl true - def handle_event("next-page", _params, socket) do - tp = socket.assigns.table_params - {:noreply, push_patch(socket, to: TableParams.to_path(%{tp | page: tp.page + 1}, "/teams"))} - end - - @impl true - def handle_event("prev-page", _params, socket) do - tp = socket.assigns.table_params - - {:noreply, - push_patch(socket, to: TableParams.to_path(%{tp | page: max(tp.page - 1, 1)}, "/teams"))} - end - defp load_teams(socket) do tp = socket.assigns.table_params diff --git a/lib/tracker_web/live/team_live/show.ex b/lib/tracker_web/live/team_live/show.ex index 40997bd..8a525b5 100644 --- a/lib/tracker_web/live/team_live/show.ex +++ b/lib/tracker_web/live/team_live/show.ex @@ -84,6 +84,7 @@ defmodule TrackerWeb.TeamLive.Show do next_path={ TableParams.page_path(@table_params, @current_page + 1, "/teams/#{@team.short_name}") } + anchor="team-packages" /> """ end @@ -122,34 +123,6 @@ defmodule TrackerWeb.TeamLive.Show do )} end - @impl true - def handle_event("next-page", _params, socket) do - tp = socket.assigns.table_params - - {:noreply, - push_patch(socket, - to: - TableParams.to_path( - %{tp | page: tp.page + 1}, - "/teams/#{socket.assigns.team.short_name}" - ) - )} - end - - @impl true - def handle_event("prev-page", _params, socket) do - tp = socket.assigns.table_params - - {:noreply, - push_patch(socket, - to: - TableParams.to_path( - %{tp | page: max(tp.page - 1, 1)}, - "/teams/#{socket.assigns.team.short_name}" - ) - )} - end - defp reload_packages(socket) do tp = socket.assigns.table_params channel_id = TrackerWeb.Lens.channel_id(socket.assigns.lens) diff --git a/test/tracker_web/components/pagination_test.exs b/test/tracker_web/components/pagination_test.exs index ed451fe..ca07b7e 100644 --- a/test/tracker_web/components/pagination_test.exs +++ b/test/tracker_web/components/pagination_test.exs @@ -7,7 +7,7 @@ defmodule TrackerWeb.PaginationTest do alias TrackerWeb.Pagination describe "controls/1" do - test "renders page info and buttons when total_pages > 1" do + test "renders page info and prev/next links when total_pages > 1" do assigns = %{} html = @@ -17,12 +17,15 @@ defmodule TrackerWeb.PaginationTest do current_page={2} has_prev_page?={true} has_next_page?={true} + prev_path="/items?page=1" + next_path="/items?page=3" + anchor="items" /> """) assert html =~ "Page 2 of 3" - assert html =~ "prev-page" - assert html =~ "next-page" + assert html =~ ~s(href="/items?page=1#items") + assert html =~ ~s(href="/items?page=3#items") end test "hidden when total_pages <= 1" do @@ -33,45 +36,14 @@ defmodule TrackerWeb.PaginationTest do """) refute html =~ "Page" - refute html =~ "prev-page" - end - - test "disables prev button on first page" do - assigns = %{} - - html = - rendered_to_string(~H""" - - """) - - assert html =~ "disabled" - end - - test "disables next button on last page" do - assigns = %{} - - html = - rendered_to_string(~H""" - - """) - - # Both buttons present, next should be disabled - assert html =~ "prev-page" - assert html =~ "next-page" + refute html =~ "href" end test "renders page number without total when total_pages is nil" do @@ -84,13 +56,14 @@ defmodule TrackerWeb.PaginationTest do current_page={2} has_prev_page?={true} has_next_page?={true} + prev_path="/items?page=1" + next_path="/items?page=3" + anchor="items" /> """) assert html =~ "Page 2" refute html =~ "Page 2 of" - assert html =~ "prev-page" - assert html =~ "next-page" end test "hidden when total_pages is nil and no neighboring pages" do @@ -103,51 +76,57 @@ defmodule TrackerWeb.PaginationTest do current_page={1} has_prev_page?={false} has_next_page?={false} + prev_path="/items?page=1" + next_path="/items?page=2" + anchor="items" /> """) refute html =~ "Page" - refute html =~ "prev-page" + refute html =~ "href" end - test "renders prev/next as links when prev_path and next_path are given" do + test "disabled prev/next render as non-link spans" do assigns = %{} html = rendered_to_string(~H""" """) - assert html =~ ~s(href="/items?page=1") - assert html =~ ~s(href="/items?page=3") + assert html =~ ~s(href="/items?page=2#items") + refute html =~ ~s(href="/items?page=1) + assert html =~ ~s(aria-disabled="true") end - test "disabled prev/next render as non-link spans when paths are given" do + test "carries the anchor hook so JS navigation lands on the list too" do assigns = %{} html = rendered_to_string(~H""" """) - # next is enabled — should be an anchor - assert html =~ ~s(href="/items?page=2") - # prev is disabled — should NOT be an anchor to that URL - refute html =~ ~s(href="/items?page=1") + assert html =~ ~s(phx-hook="PageAnchor") + assert html =~ ~s(data-anchor="items") + assert html =~ ~s(data-page="2") + assert html =~ ~s(id="pagination-items") end end end diff --git a/test/tracker_web/live/package_live/index_test.exs b/test/tracker_web/live/package_live/index_test.exs index 2d371bf..94fff61 100644 --- a/test/tracker_web/live/package_live/index_test.exs +++ b/test/tracker_web/live/package_live/index_test.exs @@ -392,7 +392,7 @@ defmodule TrackerWeb.PackageLive.IndexTest do assert html =~ "Page 1" refute html =~ "Page 1 of" - assert html =~ ~s(href="/packages?page=2") + assert html =~ ~s(href="/packages?page=2#packages") end test "later pages keep prev/next without a total", %{conn: conn} do -- 2.51.2