Replace jumpMenu controls with menu-button components

module/VuFind/src/VuFind/AjaxHandler/GetSearchResults.php

@@ -5,7 +5,7 @@
  *
  * PHP version 8
  *
- * Copyright (C) The National Library of Finland 2023.
+ * Copyright (C) The National Library of Finland 2023-2024.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2,
@@ -96,6 +96,14 @@ class GetSearchResults extends \VuFind\AjaxHandler\AbstractBase implements
                 'aria-live' => 'polite',
             ],
         ],
+        '.js-search-sort' => [
+            'method' => 'renderSort',
+            'target' => 'inner',
+        ],
+        '.js-search-limit' => [
+            'method' => 'renderLimit',
+            'target' => 'inner',
+        ],
         '.js-result-list' => [
             'method' => 'renderResults',
             'target' => 'outer',
@@ -334,6 +342,46 @@ protected function renderSearchStats(ParamsHelper $requestParams, Results $resul
         return $this->translate($statsKey, $transParams);
     }
 
+    /**
+     * Render sort control
+     *
+     * @param Params  $params  Request params
+     * @param Results $results Search results
+     *
+     * @return ?string
+     */
+    protected function renderSort(Params $params, Results $results): ?string
+    {
+        $params = $results->getParams();
+        return $this->renderer->render(
+            'search/controls/sort.phtml',
+            compact(
+                'results',
+                'params',
+            )
+        );
+    }
+
+    /**
+     * Render limit control
+     *
+     * @param Params  $params  Request params
+     * @param Results $results Search results
+     *
+     * @return ?string
+     */
+    protected function renderLimit(Params $params, Results $results): ?string
+    {
+        $params = $results->getParams();
+        return $this->renderer->render(
+            'search/controls/limit.phtml',
+            compact(
+                'results',
+                'params',
+            )
+        );
+    }
+
     /**
      * Render analytics
      *

module/VuFind/src/VuFindTest/Feature/SearchSortTrait.php

@@ -49,7 +49,7 @@ trait SearchSortTrait
      *
      * @var string
      */
-    protected $sortControlSelector = '#sort_options_1';
+    protected $sortControlSelector = '.search__sort';
 
     /**
      * VuFind default sort options
@@ -97,7 +97,15 @@ protected function assertResultTitles(Element $page, int $count, string $first,
      */
     protected function sortResults(Element $page, string $value): void
     {
-        $this->findCssAndSetValue($page, $this->sortControlSelector, $value);
+        $sortControl = $this->clickCss($page, $this->sortControlSelector);
+        $links = $sortControl->findAll('css', 'a');
+        foreach ($links as $link) {
+            if ($link->getText() === $value) {
+                $link->click();
+                return;
+            }
+        }
+        throw new \Exception("Sort link for '$value' not found");
     }
 
     /**
@@ -110,7 +118,7 @@ protected function sortResults(Element $page, string $value): void
      */
     protected function assertSelectedSort(Element $page, string $active): void
     {
-        $sort = $this->findCss($page, $this->sortControlSelector);
-        $this->assertEquals((string)$active, $sort->getValue());
+        $sort = $this->findCss($page, $this->sortControlSelector . ' li.active a');
+        $this->assertEquals($active, $sort->getText());
     }
 }

module/VuFind/tests/integration-tests/src/VuFindTest/Mink/IlsActionsTest.php

@@ -609,7 +609,8 @@ public function testLoanHistory(): void
                 $this->findCssAndGetText($page, 'ul.record-list li a.title', null, $index)
             );
         }
-        $this->clickCss($page, '#sort_options_1 option', null, 2);
+        $this->clickCss($page, '.list__sort button.dropdown-toggle');
+        $this->clickCss($page, '.list__sort li a', null, 2);
         $this->waitForPageLoad($page);
         foreach (array_reverse($titles) as $index => $title) {
             $this->assertEquals(

module/VuFind/tests/integration-tests/src/VuFindTest/Mink/LibraryCardsTest.php

@@ -207,8 +207,8 @@ public function testSwitchingCards(): void
         $this->waitForPageLoad($page);
 
         // Confirm the presence of a library card selector on the page:
-        $firstCard = $this->findCss($page, '#library_card option:nth-child(1)');
-        $secondCard = $this->findCss($page, '#library_card option:nth-child(2)');
+        $firstCard = $this->findCss($page, '.select-library-card li:nth-child(1) a');
+        $secondCard = $this->findCss($page, '.select-library-card li:nth-child(2) a');
         $card2Value = $secondCard->getValue();
         $this->assertEquals('card 1', $firstCard->getText());
         $this->assertEquals('card 2', $secondCard->getText());

module/VuFind/tests/integration-tests/src/VuFindTest/Mink/RecordActionsTest.php

@@ -327,7 +327,7 @@ public function testTagSearch(): void
         // Now perform the search:
         $page = $this->performSearch('five', 'tag');
         $this->assertResultTitles($page, 3, 'Dewey browse test', '<HTML> The Basics');
-        $this->assertSelectedSort($page, 'title');
+        $this->assertSelectedSort($page, 'Title');
     }
 
     /**
@@ -338,17 +338,16 @@ public function testTagSearch(): void
     public static function getTagSearchSortData(): array
     {
         return [
-            [1, 'author', 'Fake Record 1 with multiple relators/', 'Dewey browse test'],
-            [2, 'year DESC', '<HTML> The Basics', 'Fake Record 1 with multiple relators/'],
-            [3, 'year', 'Fake Record 1 with multiple relators/', '<HTML> The Basics'],
+            ['Author', 'Fake Record 1 with multiple relators/', 'Dewey browse test'],
+            ['Date Descending', '<HTML> The Basics', 'Fake Record 1 with multiple relators/'],
+            ['Date Ascending', 'Fake Record 1 with multiple relators/', '<HTML> The Basics'],
         ];
     }
 
     /**
      * Test sorting the tag search results.
      *
-     * @param int    $index         Sort drop-down index to test
-     * @param string $expectedSort  Expected sort value at $index
+     * @param string $sort          Sort option to use
      * @param string $expectedFirst Expected first title after sorting
      * @param string $expectedLast  Expected last title after sorting
      *
@@ -359,16 +358,15 @@ public static function getTagSearchSortData(): array
      * @depends testTagSearch
      */
     public function testTagSearchSort(
-        int $index,
-        string $expectedSort,
+        string $sort,
         string $expectedFirst,
         string $expectedLast
     ): void {
         $page = $this->performSearch('five', 'tag');
-        $this->clickCss($page, $this->sortControlSelector . ' option', null, $index);
+        $this->sortResults($page, $sort);
         $this->waitForPageLoad($page);
         $this->assertResultTitles($page, 3, $expectedFirst, $expectedLast);
-        $this->assertSelectedSort($page, $expectedSort);
+        $this->assertSelectedSort($page, $sort);
     }
 
     /**
@@ -734,26 +732,24 @@ public function testRating($allowRemove): void
         $this->findCss($page, 'form.comment-form a');
         $this->clickCss($page, 'form.comment-form .btn-primary');
         // Check result (wait for the value to update):
-        $this->assertEqualsWithTimeout(
-            [1, '80'],
-            function () use ($page, $checked) {
-                $inputs = $page->findAll('css', $checked);
+        $ratingCallback = function () use ($page, $checked) {
+            $inputs = $page->findAll('css', $checked);
+            // getValue can fail in the middle of update, so wrap in try..catch:
+            try {
                 return [count($inputs), $inputs ? $inputs[0]->getValue() : null];
+            } catch (\Exception $e) {
+                return [null, null];
             }
-        );
+        };
+
+        $this->assertEqualsWithTimeout([1, '80'], $ratingCallback);
         if ($allowRemove) {
             // Clear rating when adding another comment
             $this->findCssAndSetValue($page, 'form.comment-form [name="comment"]', 'two');
             $this->clickCss($page, 'form.comment-form a');
             $this->clickCss($page, 'form.comment-form .btn-primary');
             // Check result (wait for the value to update):
-            $this->assertEqualsWithTimeout(
-                [1, '70'],
-                function () use ($page, $checked) {
-                    $inputs = $page->findAll('css', $checked);
-                    return [count($inputs), $inputs ? $inputs[0]->getValue() : null];
-                }
-            );
+            $this->assertEqualsWithTimeout([1, '70'], $ratingCallback);
         } else {
             // Check that the "Clear" link is no longer available:
             $this->unFindCss($page, 'form.comment-form a');

module/VuFind/tests/integration-tests/src/VuFindTest/Mink/SavedSearchesTest.php

@@ -47,6 +47,13 @@ final class SavedSearchesTest extends \VuFindTest\Integration\MinkTestCase
     use \VuFindTest\Feature\LiveDatabaseTrait;
     use \VuFindTest\Feature\UserCreationTrait;
 
+    /**
+     * Selector for the schedule control
+     *
+     * @var string
+     */
+    protected $scheduleSelector = '.search__notify';
+
     /**
      * Standard setup method.
      *
@@ -114,18 +121,13 @@ protected function assertSavedSearchList(array $expected, Element $page): void
     {
         // Pull all the links from the search history table and format them into
         // a string:
-        $saved = $page->findAll('css', '#saved-searches td a');
+        $saved = $page->findAll('css', '#saved-searches a.history-entry');
         $callback = function ($link) {
             return trim($link->getText());
         };
         $linkText = implode("\n", array_map($callback, $saved));
 
-        // Each expected search link should have a corresponding Delete link; create
-        // an expectation accordingly:
-        $expectedCallback = function ($link) {
-            return "$link\nDelete";
-        };
-        $expectedLinkText = implode("\n", array_map($expectedCallback, $expected));
+        $expectedLinkText = implode("\n", $expected);
 
         // Compare the expected and actual strings:
         $this->assertEquals($expectedLinkText, $linkText);
@@ -311,39 +313,39 @@ public function testNotificationSettings(): void
         $this->findAndAssertLink($page, 'Search History')->click();
 
         // By default, there should be no alert option at all:
-        $scheduleSelector = 'select[name="schedule"]';
-        $this->assertNull($page->find('css', $scheduleSelector));
+        $this->assertNull($page->find('css', $this->scheduleSelector));
 
         // Now reconfigure to allow notifications, and refresh the page:
         $page = $this->activateNotifications();
 
         // Now there should be two alert options visible (one in saved, one in
         // unsaved):
-        $this->assertCount(2, $page->findAll('css', $scheduleSelector));
+        $this->assertCount(2, $page->findAll('css', $this->scheduleSelector));
         $this->assertCount(
             1,
-            $page->findAll('css', '#recent-searches ' . $scheduleSelector)
+            $page->findAll('css', '#recent-searches ' . $this->scheduleSelector)
         );
         $this->assertCount(
             1,
-            $page->findAll('css', '#saved-searches ' . $scheduleSelector)
+            $page->findAll('css', '#saved-searches ' . $this->scheduleSelector)
         );
 
         // At this point, our journals search should be in the unsaved list; let's
         // set it up for alerts and confirm that this auto-saves it.
-        $select = $this->findCss($page, '#recent-searches ' . $scheduleSelector);
-        $select->selectOption(7);
+        $this->clickCss($page, '#recent-searches ' . $this->scheduleSelector);
+        $this->waitForPageLoad($page);
+        $this->findCss($page, '#recent-searches ' . $this->scheduleSelector)->findLink('Weekly')->click();
         $this->waitForPageLoad($page);
         $this->assertCount(
             2,
-            $page->findAll('css', '#saved-searches ' . $scheduleSelector)
+            $page->findAll('css', '#saved-searches ' . $this->scheduleSelector)
         );
 
         // Now let's delete the saved search and confirm that this clears the
         // alert subscription.
         $this->findAndAssertLink($page, 'Delete')->click();
         $this->waitForPageLoad($page);
-        $select = $this->findCss($page, '#recent-searches ' . $scheduleSelector);
+        $select = $this->findCss($page, '#recent-searches ' . $this->scheduleSelector);
         $this->assertEquals(0, $select->getValue());
     }
 
@@ -377,9 +379,9 @@ public function testNotificationsInSearchToolbar()
         $this->waitForPageLoad($page);
 
         // We should now be on a page with a schedule selector; let's pick something:
-        $scheduleSelector = 'select[name="schedule"]';
-        $select = $this->findCss($page, $scheduleSelector);
-        $select->selectOption(7);
+        $this->clickCss($page, $this->scheduleSelector);
+        $this->waitForPageLoad($page);
+        $this->findCss($page, $this->scheduleSelector)->findLink('Weekly')->click();
         $this->waitForPageLoad($page);
 
         // Let's confirm that if we repeat the search, the alert will now be set:
@@ -415,6 +417,11 @@ public function testNotificationsInSearchToolbarDeduplication()
 
         // We should now be on a page with a schedule selector; because of the
         // setting we set in the previous test, and with login deduplication, we
+        // should now see the "Weekly" option already selected:
+        $this->assertEquals(
+            'Weekly',
+            $this->findCss($page, $this->scheduleSelector . ' button')->getText()
+        );
         // should now see the "7" option already selected:
         $scheduleSelector = 'select[name="schedule"]';
         $this->assertEquals(7, $this->findCssAndGetValue($page, $scheduleSelector));
@@ -439,20 +446,21 @@ public function testNotificationsInSearchHistoryDeduplication()
         $this->waitForPageLoad($page);
 
         // Now there should be one alert option visible (in unsaved):
-        $scheduleSelector = 'select[name="schedule"]';
-        $this->assertCount(1, $page->findAll('css', $scheduleSelector));
+        $this->assertCount(1, $page->findAll('css', $this->scheduleSelector));
         $this->assertCount(
             1,
-            $page->findAll('css', '#recent-searches ' . $scheduleSelector)
+            $page->findAll('css', '#recent-searches ' . $this->scheduleSelector)
         );
         $this->assertCount(
             0,
-            $page->findAll('css', '#saved-searches ' . $scheduleSelector)
+            $page->findAll('css', '#saved-searches ' . $this->scheduleSelector)
         );
 
         // Let's set up our search for alerts and make sure it's handled correctly:
-        $select = $this->findCss($page, '#recent-searches ' . $scheduleSelector);
-        $select->selectOption(1);
+        $select = $this->findCss($page, '#recent-searches ' . $this->scheduleSelector);
+        $select->click();
+        $this->waitForPageLoad($page);
+        $select->findLink('Daily')->click();
         $this->waitForPageLoad($page);
 
         // We should now be prompted to log in:
@@ -469,8 +477,13 @@ public function testNotificationsInSearchHistoryDeduplication()
         $this->assertSavedSearchList(['employment', 'test'], $page);
         $this->assertCount(
             2,
-            $page->findAll('css', '#saved-searches ' . $scheduleSelector)
+            $page->findAll('css', '#saved-searches ' . $this->scheduleSelector)
+        );
+        $this->assertEquals(
+            'Daily',
+            $this->findCss($page, $this->scheduleSelector . ' button')->getText()
         );
+        $scheduleSelector = 'select[name="schedule"]';
         $this->assertEquals(1, $this->findCssAndGetValue($page, $scheduleSelector));
     }