[#55392] project list: ensure pagination reset on sort change

opf · Sep 20, 2024 · fba16ed · fba16ed
1 parent ceabb80
commit fba16ed
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 74 deletions.
diff --git a/app/components/projects/configure_view_modal_component.html.erb b/app/components/projects/configure_view_modal_component.html.erb
@@ -1,7 +1,6 @@
 <%= render(Primer::Alpha::Dialog.new(title: t(:'queries.configure_view.heading'),
                                      size: :large,
                                      id: MODAL_ID,
-                                     data: { controller: "pagination-reset-on-sort", application_target: "dynamic" },
                                      # Hack to give the draggable autcompleter (ng-select) bound to the dialog
                                      # enough height to display all options.
                                      # This is necessary as long as ng-select does not support popovers.
@@ -11,15 +10,11 @@
     <%= primer_form_with(
           url: projects_path,
           id: QUERY_FORM_ID,
-          data: {
-            "turbo-stream" => false,
-            "pagination-reset-on-sort-target" => "form",
-            action: "pagination-reset-on-sort#submit"
-          },
+          data: { "turbo-stream" => false },
           method: :get
         ) do |form| %>
       <% helpers.projects_query_params.except(:columns, :sortBy).each do |name, value| %>
-        <%= hidden_field_tag name, value, data: {"pagination-reset-on-sort-target" => name} %>
+        <%= hidden_field_tag name, value, data: {"sort-by-config-target" => name} %>
       <% end %>
       <%= render(Primer::Alpha::TabPanels.new(label: "label")) do |tab_panel| %>
         <% tab_panel.with_tab(selected: true, id: "tab-selects--columns") do |tab| %>

diff --git a/app/components/queries/sort_by_component.html.erb b/app/components/queries/sort_by_component.html.erb
@@ -1,9 +1,7 @@
 <%= render(Primer::Beta::Heading.new(tag: :h5)) { I18n.t('queries.configure_view.sort_by.automatic.heading') } %>
 <%= render(Primer::Beta::Text.new(font_size: :small, color: :subtle)) { I18n.t('queries.configure_view.sort_by.automatic.description', plural: queried_model_name.plural) } %>
 <div data-controller="sort-by-config" data-application-target="dynamic">
-  <%= hidden_field_tag :sortBy, current_orders, data: {
-    "sort-by-config-target" => "sortByField",
-    "pagination-reset-on-sort-target" => "sortByField" } %>
+  <%= hidden_field_tag :sortBy, current_orders, data: { "sort-by-config-target" => "sortByField" } %>
   <%= render(Primer::OpenProject::FlexLayout.new(data: { "sort-by-config-target" => "inputRowContainer" })) do |layout| %>
     <% order_limit.times do |i| %>
       <% layout.with_row(mt: 3, data: { "sort-by-config-target" => "inputRow" }) do %>

diff --git a/frontend/src/stimulus/controllers/dynamic/pagination-reset-on-sort.controller.ts b/frontend/src/stimulus/controllers/dynamic/pagination-reset-on-sort.controller.ts
diff --git a/frontend/src/stimulus/controllers/dynamic/sort-by-config.controller.ts b/frontend/src/stimulus/controllers/dynamic/sort-by-config.controller.ts
@@ -50,13 +50,21 @@ export default class SortByConfigController extends Controller {
   declare readonly inputRowTargets:HTMLElement[];
   declare readonly inputRowContainerTarget:HTMLElement;
 
+  declare parentForm:HTMLFormElement | null;
+  declare pageTarget:HTMLInputElement;
+
+  declare initialSortBy:string;
+
   connect():void {
     this.inputRowTargets.forEach((row) => {
       this.manageRow(row);
     });
 
     this.displayNewFieldSelectorIfNeeded();
     this.disableSelectedFieldsForOtherSelects();
+
+    this.initialSortBy = this.sortByFieldTarget.value;
+    this.registerPaginationResetHandler();
   }
 
   buildSortJson():string {
@@ -69,6 +77,43 @@ export default class SortByConfigController extends Controller {
     return JSON.stringify(compact(filters));
   }
 
+  // Tries to find the parent form in the DOM. If present and the form contains a `page` field marked
+  // with the proper target, will register a handler to trigger a pagination reset when the sorting
+  // changes.
+  registerPaginationResetHandler():void {
+    this.parentForm = this.sortByFieldTarget.closest('form');
+
+    if (this.parentForm) {
+      this.pageTarget = this.parentForm.querySelector('input[data-sort-by-config-target="page"]') as HTMLInputElement;
+
+      if (this.pageTarget) {
+        this.parentForm.addEventListener('submit', this.onFormSubmit.bind(this));
+      }
+    }
+  }
+
+  onFormSubmit(event:SubmitEvent):void {
+    if (!this.parentForm || !this.pageTarget) { return; }
+    event.preventDefault();
+
+    this.resetPaginationIfSortingChanged();
+
+    this.parentForm.submit();
+  }
+
+  // When the sorting criteria changes, reset the pagination to the first page.
+  // This leads to a better UX since the previous position in the pagination becomes
+  // obsolete when the underlying collection changes due to sorting adjustments. It is better to start
+  // from a fresh perspective on the data.
+  resetPaginationIfSortingChanged():void {
+    const currentSelection = this.sortByFieldTarget.value;
+
+    if (this.initialSortBy !== currentSelection) {
+      // Reset the pagination:
+      this.pageTarget.value = '';
+    }
+  }
+
   fieldChanged(event:Event):void {
     const target = event.target as HTMLElement;
     const row = target.closest('div[data-sort-by-config-target="inputRow"]') as HTMLElement;