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.

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

Replace jumpMenu controls with menu-button components #2814

Draft
wants to merge 47 commits into
base: dev
Choose a base branch
from
Draft
Changes from 8 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
d064beb
Add showSelectedValue option in menu-button component.
demiankatz Mar 31, 2023
bbe193c
Convert sort control to use menu-button.
demiankatz Mar 31, 2023
d97e578
Fix style.
demiankatz Mar 31, 2023
2706988
Convert limit control.
demiankatz Mar 31, 2023
9fa8233
php-cs-fixer
demiankatz Mar 31, 2023
c8ff113
Merge branch 'dev' into jumpmenu-to-menu-button
demiankatz Apr 4, 2023
0e2711b
Add support for custom link attributes.
demiankatz Apr 4, 2023
88cb036
Eliminate library card jump menu.
demiankatz Apr 4, 2023
7854e41
Fix styles.
demiankatz Apr 4, 2023
694dc0c
Style tweak.
demiankatz Apr 4, 2023
a621742
Refactor myresearch sort.
demiankatz Apr 4, 2023
626f46e
Allow no label.
demiankatz Apr 4, 2023
72fe14b
Refactor scheduled search control.
demiankatz Apr 4, 2023
9590f21
Finish jumpmenu conversion.
demiankatz Apr 4, 2023
01df62f
Remove jumpMenu logic.
demiankatz Apr 4, 2023
d496be6
style: apply button styles and alignment to new search controls.
crhallberg Apr 11, 2023
f6fca3e
Merge branch 'dev' into jumpmenu-to-menu-button
demiankatz Apr 12, 2023
0bb8fac
Separate selected value from label.
demiankatz Apr 12, 2023
83db603
style: visual and code styles.
crhallberg Apr 12, 2023
6cc486a
Revert accidental change.
demiankatz Apr 13, 2023
b7f3943
lessToSass
demiankatz Apr 13, 2023
0970aa1
Space out icon with CSS instead of actual space.
demiankatz Apr 13, 2023
1825b0f
Fix SearchLimitTest and make some associated fixes/improvements.
demiankatz Apr 13, 2023
09c6326
Fix SavedSearchesTest.
demiankatz Apr 13, 2023
607cb34
php-cs-fixer
demiankatz Apr 13, 2023
625c694
Merge branch 'dev' into jumpmenu-to-menu-button
demiankatz Apr 13, 2023
ad4af22
Merge branch 'dev' into jumpmenu-to-menu-button
demiankatz Jun 21, 2023
b713db9
Merge remote-tracking branch 'origin/dev' into jumpmenu-to-menu-button
crhallberg Jul 25, 2023
307197f
Merge branch 'dev' into jumpmenu-to-menu-button-2
EreMaijala Jan 11, 2024
7133452
Fix search controls, stack vertically.
EreMaijala Jan 11, 2024
63bed40
Fix alignment and spacing issues.
EreMaijala Jan 11, 2024
6ebf0f5
Fix empty label and white-space decoration issue in search history.
EreMaijala Jan 11, 2024
aa5824c
Fix Mink tests.
EreMaijala Jan 11, 2024
89014c8
Improve dropdown alignment and nav menu button style.
EreMaijala Jan 11, 2024
46be96d
Fix code style.
EreMaijala Jan 11, 2024
2144891
Merge pull request #29 from EreMaijala/jumpmenu-to-menu-button
demiankatz Jan 11, 2024
fd7264e
Merge remote-tracking branch 'origin/dev' into jumpmenu-to-menu-button
crhallberg Feb 6, 2024
d59134f
fix: style search controls to take up enough room.
crhallberg Feb 6, 2024
26171bc
lessToSass.
demiankatz Feb 8, 2024
4acca55
Merge branch 'dev' into jumpmenu-to-menu-button
demiankatz Feb 8, 2024
b0e7cab
Restore missing definition.
demiankatz Feb 8, 2024
f6a9570
Merge remote-tracking branch 'origin/dev' into jumpmenu-to-menu-button
crhallberg Feb 27, 2024
b0cd316
Merge remote-tracking branch 'origin/dev' into jumpmenu-to-menu-button
crhallberg Feb 28, 2024
c7dfee4
Use clickCss where possible.
demiankatz Feb 28, 2024
9ca7d95
Merge remote-tracking branch 'origin/dev' into jumpmenu-to-menu-button
crhallberg Mar 25, 2024
cf7d254
Merge remote-tracking branch 'origin/dev' into jumpmenu-to-menu-button
crhallberg May 7, 2024
2e0e7e0
chore: less
crhallberg May 7, 2024
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
50 changes: 49 additions & 1 deletion module/VuFind/src/VuFind/AjaxHandler/GetSearchResults.php
Original file line number Diff line number Diff line change
@@ -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,
@@ -151,6 +151,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',
@@ -385,6 +393,46 @@ protected function renderSearchStats(Params $params, Results $results): ?string
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
*
16 changes: 12 additions & 4 deletions module/VuFind/src/VuFindTest/Feature/SearchSortTrait.php
Original file line number Diff line number Diff line change
@@ -49,7 +49,7 @@ trait SearchSortTrait
*
* @var string
*/
protected $sortControlSelector = '#sort_options_1';
protected $sortControlSelector = '.search__sort';

/**
* VuFind default sort options
@@ -92,7 +92,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");
}

/**
@@ -105,7 +113,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());
}
}
Original file line number Diff line number Diff line change
@@ -610,7 +610,8 @@ public function testLoanHistory(): void
$this->findCss($page, 'ul.record-list li a.title', null, $index)->getText()
);
}
$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(
Original file line number Diff line number Diff line change
@@ -210,9 +210,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)');
$card2Value = $secondCard->getValue();
$firstCard = $this->findCss($page, '.select-library-card li:nth-child(1) a');
$secondCard = $this->findCss($page, '.select-library-card li:nth-child(2) a');
$this->assertEquals('card 1', $firstCard->getText());
$this->assertEquals('card 2', $secondCard->getText());

@@ -222,9 +221,8 @@ public function testSwitchingCards(): void
$this->findCss($page, '.catalog-profile tr:nth-child(1) td:nth-child(2)')->getText()
);

// Switch to the second card; we can't currently use findCssAndSetValue() here because
// it conflicts with the behavior of jumpMenu.
$this->findCss($page, '#library_card')->setValue($card2Value);
// Switch to the second card
$this->clickCss($page, '.select-library-card li:nth-child(2) a');
$this->waitForPageLoad($page);

// Check that the appropriate username is reflected in the output:
Original file line number Diff line number Diff line change
@@ -334,7 +334,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');
}

/**
@@ -345,17 +345,16 @@ public function testTagSearch(): void
public 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
*
@@ -366,16 +365,15 @@ public 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);
}

/**
@@ -758,26 +756,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->findCss($page, 'form.comment-form [name="comment"]')->setValue('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');
Original file line number Diff line number Diff line change
@@ -338,7 +338,7 @@ public function testNotificationSettings(): void
// set it up for alerts and confirm that this auto-saves it.
$this->findCss($page, '#recent-searches ' . $this->scheduleSelector)->click();
$this->waitForPageLoad($page);
$this->findCss($page, '#recent-searches ' . $this->scheduleSelector)->findLink("Weekly")->click();
$this->findCss($page, '#recent-searches ' . $this->scheduleSelector)->findLink('Weekly')->click();
$this->waitForPageLoad($page);
$this->assertCount(
2,
@@ -385,7 +385,7 @@ public function testNotificationsInSearchToolbar()
// We should now be on a page with a schedule selector; let's pick something:
$this->findCss($page, $this->scheduleSelector)->click();
$this->waitForPageLoad($page);
$this->findCss($page, $this->scheduleSelector)->findLink("Weekly")->click();
$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:
@@ -424,7 +424,7 @@ public function testNotificationsInSearchToolbarDeduplication()
// setting we set in the previous test, and with login deduplication, we
// should now see the "Weekly" option already selected:
$this->assertEquals(
"Weekly",
'Weekly',
$this->findCss($page, $this->scheduleSelector . ' button')->getText()
);
}
@@ -462,7 +462,7 @@ public function testNotificationsInSearchHistoryDeduplication()
$select = $this->findCss($page, '#recent-searches ' . $this->scheduleSelector);
$select->click();
$this->waitForPageLoad($page);
$select->findLink("Daily")->click();
$select->findLink('Daily')->click();
$this->waitForPageLoad($page);

// We should now be prompted to log in:
@@ -482,7 +482,7 @@ public function testNotificationsInSearchHistoryDeduplication()
$page->findAll('css', '#saved-searches ' . $this->scheduleSelector)
);
$this->assertEquals(
"Daily",
'Daily',
$this->findCss($page, $this->scheduleSelector . ' button')->getText()
);
}
Original file line number Diff line number Diff line change
@@ -248,7 +248,7 @@ protected function facetListProcedure(Element $page, int $limit, bool $exclusion
public function testApplyFacet(): void
{
$page = $this->performSearch('building:weird_ids.mrc');
$this->sortResults($page, 'title');
$this->sortResults($page, 'Title');
$this->waitForPageLoad($page);

// Confirm that we are NOT using the AJAX sidebar:
@@ -259,7 +259,7 @@ public function testApplyFacet(): void
$this->facetApplyProcedure($page);

// Verify that sort order is still correct:
$this->assertSelectedSort($page, 'title');
$this->assertSelectedSort($page, 'Title');
}

/**
@@ -279,7 +279,7 @@ public function testApplyFacetDeferred(): void
]
);
$page = $this->performSearch('building:weird_ids.mrc');
$this->sortResults($page, 'title');
$this->sortResults($page, 'Title');
$this->waitForPageLoad($page);

// Confirm that we ARE using the AJAX sidebar:
@@ -290,7 +290,7 @@ public function testApplyFacetDeferred(): void
$this->facetApplyProcedure($page);

// Verify that sort order is still correct:
$this->assertSelectedSort($page, 'title');
$this->assertSelectedSort($page, 'Title');
}

/**
@@ -470,14 +470,14 @@ public function testHierarchicalFacets(): void
);
// Do a search and verify that sort order is maintained:
$page = $this->performSearch('building:"hierarchy.mrc"');
$this->sortResults($page, 'title');
$this->sortResults($page, 'Title');
$this->waitForPageLoad($page);
$this->clickHierarchicalFacet($page);
$this->assertSelectedSort($page, 'title');
$this->assertSelectedSort($page, 'Title');
// Remove the filter:
$this->clickCss($page, $this->activeFilterSelector);
$this->waitForPageLoad($page);
$this->assertSelectedSort($page, 'title');
$this->assertSelectedSort($page, 'Title');
}

/**
Original file line number Diff line number Diff line change
@@ -232,7 +232,8 @@ public function testLimitChange(): void
$this->assertResultTitles($page, 'Test Publication 20021', 'Test Publication 20040', 20);

// Change limit and verify:
$this->clickCss($page, $this->limitControlSelector . ' option', null, 1);
$item = $this->clickCss($page, $this->limitControlSelector . ' li', null, 1);
$this->clickCss($item, 'a');
$this->waitForPageLoad($page);
// Check expected first and last record (page should be reset):
$this->assertResultTitles($page, 'Test Publication 20001', 'Test Publication 20040', 40);
Original file line number Diff line number Diff line change
@@ -33,8 +33,6 @@

use Behat\Mink\Element\Element;

use function count;

/**
* Test for sorting of search results.
*
@@ -57,7 +55,7 @@ class SearchSortTest extends \VuFindTest\Integration\MinkTestCase
public function testInvalidSort(): void
{
$page = $this->setUpSearch('foobar', 'relevance');
$this->assertSortControl($page, 'relevance');
$this->assertSortControl($page, 'Relevance');
}

/**
@@ -68,7 +66,7 @@ public function testInvalidSort(): void
public function testDefaultSort(): void
{
$page = $this->setUpSearch('', 'title desc');
$this->assertSortControl($page, 'title desc');
$this->assertSortControl($page, 'Title Reversed');
}

/**
@@ -81,7 +79,7 @@ public function testSortChange(): void
$page = $this->setUpSearch('title', 'title');

// Check current sort:
$this->assertSortControl($page, 'title');
$this->assertSortControl($page, 'Title');

// Check expected first and last record on first page:
$this->assertResultTitles($page, 20, 'Test Publication 20001', 'Test Publication 20020');
@@ -92,10 +90,10 @@ public function testSortChange(): void
$this->assertResultTitles($page, 20, 'Test Publication 20021', 'Test Publication 20040');

// Change sort to title reversed (last option) and verify:
$this->clickCss($page, $this->sortControlSelector . ' option', null, count($this->defaultSortOptions) + 1);
$this->sortResults($page, 'Title Reversed');
$this->waitForPageLoad($page);
// Check current sort:
$this->assertSortControl($page, 'title desc');
$this->assertSortControl($page, 'Title Reversed');
// Check expected first and last record (page should be reset):
$this->assertResultTitles($page, 20, 'Test Publication 20177', 'Test Publication 201738');
// Check that url no longer contains the page parameter:
@@ -180,7 +178,7 @@ protected function setUpSearch(string $sortParam, string $default): Element
protected function assertSortControl(Element $page, string $active)
{
$this->assertSelectedSort($page, $active);
$optionElements = $page->findAll('css', $this->sortControlSelector . ' option');
$optionElements = $page->findAll('css', $this->sortControlSelector . ' li a');
$callback = function (Element $element): string {
return $element->getText();
};
4 changes: 2 additions & 2 deletions themes/bootprint3/css/compiled.css

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions themes/bootstrap3/css/compiled.css

Large diffs are not rendered by default.

Loading