diff --git a/layouts/joomla/pagination/link.php b/layouts/joomla/pagination/link.php index 5f17aee9c8a30..c938bd6aae25a 100644 --- a/layouts/joomla/pagination/link.php +++ b/layouts/joomla/pagination/link.php @@ -53,16 +53,15 @@ } if ($displayData['active']) { - if ($item->base > 0) { - $limit = 'limitstart.value=' . $item->base; - } else { - $limit = 'limitstart.value=0'; - } - - $class = 'active'; + $class = 'active'; + $linkClass = ''; if ($app->isClient('administrator')) { - $link = 'href="#" onclick="document.adminForm.' . $item->prefix . $limit . '; Joomla.submitform();return false;"'; + // The list-view script sets the limitstart field and submits the form + $app->getDocument()->getWebAssetManager()->useScript('list-view'); + + $link = 'href="#" data-limitstart-field="' . htmlspecialchars($item->prefix, ENT_COMPAT, 'UTF-8') . 'limitstart" data-limitstart="' . (int) $item->base . '"'; + $linkClass = ' js-grid-pagination-link'; } elseif ($app->isClient('site')) { $link = 'href="' . $item->link . '"'; } @@ -73,7 +72,7 @@ ?>
  • - class="page-link"> + class="page-link">
  • diff --git a/libraries/src/HTML/Helpers/Grid.php b/libraries/src/HTML/Helpers/Grid.php index 15319d107b7fe..efba21631fe86 100644 --- a/libraries/src/HTML/Helpers/Grid.php +++ b/libraries/src/HTML/Helpers/Grid.php @@ -89,9 +89,23 @@ public static function sort($title, $order, $direction = 'asc', $selected = '', */ public static function checkall($name = 'checkall-toggle', $action = 'Joomla.checkAll(this)') { - Factory::getApplication()->getDocument()->getWebAssetManager()->useScript('core'); + $wa = Factory::getApplication()->getDocument()->getWebAssetManager(); - return ''; + // The default action is handled by the list-view script, a custom action stays an inline handler + if ($action === 'Joomla.checkAll(this)') { + $wa->useScript('list-view'); + + $class = ' js-grid-item-check-all'; + $action = ''; + } else { + $wa->useScript('core'); + + $class = ''; + $action = ' onclick="' . $action . '"'; + } + + return ''; } /** @@ -206,12 +220,14 @@ public static function state($filterState = '*', $published = 'JPUBLISHED', $unp $state['T'] = Text::_($trashed); } + Factory::getApplication()->getDocument()->getWebAssetManager()->useScript('list-view'); + return HTMLHelper::_( 'select.genericlist', $state, 'filter_state', [ - 'list.attr' => 'class="form-select" size="1" onchange="Joomla.submitform();"', + 'list.attr' => 'class="form-select js-grid-submit-on-change" size="1"', 'list.select' => $filterState, 'option.key' => null, ] diff --git a/libraries/src/Pagination/Pagination.php b/libraries/src/Pagination/Pagination.php index 5efee357530ee..90cf474528c33 100644 --- a/libraries/src/Pagination/Pagination.php +++ b/libraries/src/Pagination/Pagination.php @@ -576,30 +576,19 @@ public function getLimitBox() $selected = $this->viewall ? 0 : $this->limit; - // Build the select list. - if ($this->app->isClient('administrator')) { - $html = HTMLHelper::_( - 'select.genericlist', - $limits, - $this->prefix . 'limit', - 'class="form-select" onchange="Joomla.submitform();"', - 'value', - 'text', - $selected - ); - } else { - $html = HTMLHelper::_( - 'select.genericlist', - $limits, - $this->prefix . 'limit', - 'class="form-select" onchange="this.form.submit()"', - 'value', - 'text', - $selected - ); - } + // The list-view script submits the form when the limit changes + $this->app->getDocument()->getWebAssetManager()->useScript('list-view'); - return $html; + // Build the select list. + return HTMLHelper::_( + 'select.genericlist', + $limits, + $this->prefix . 'limit', + 'class="form-select js-grid-submit-on-change"', + 'value', + 'text', + $selected + ); } /** diff --git a/media_source/system/js/list-view.es6.js b/media_source/system/js/list-view.es6.js index 28d18bd6eaedb..3a4a25e79c225 100644 --- a/media_source/system/js/list-view.es6.js +++ b/media_source/system/js/list-view.es6.js @@ -89,6 +89,29 @@ function applyIsChecked(event) { } } +/* + * Submit the form of the current element, e.g. when the list limit changes + * + * @param {Event} event + */ +function gridSubmitOnChange(event) { + Joomla.submitform('', event.target.form); +} + +/* + * Go to the page of the current pagination link + * + * @param {Event} event + */ +function gridPaginationAction(event) { + event.preventDefault(); + + const { limitstartField, limitstart } = event.currentTarget.dataset; + + document.adminForm[limitstartField].value = limitstart; + Joomla.submitform(); +} + /* * Set up an interactive list elements * @@ -100,6 +123,8 @@ const setup = ({ target }) => { target.querySelectorAll('.js-grid-item-action').forEach((element) => element.addEventListener('click', gridItemAction)); target.querySelectorAll('.js-grid-item-transition-action').forEach((element) => element.addEventListener('change', gridTransitionItemAction)); target.querySelectorAll('.js-grid-button-transition-action').forEach((element) => element.addEventListener('click', gridTransitionButtonAction)); + target.querySelectorAll('.js-grid-submit-on-change').forEach((element) => element.addEventListener('change', gridSubmitOnChange)); + target.querySelectorAll('.js-grid-pagination-link').forEach((element) => element.addEventListener('click', gridPaginationAction)); }; setup({ target: document }); diff --git a/tests/Unit/Libraries/Cms/Html/HtmlGridTest.php b/tests/Unit/Libraries/Cms/Html/HtmlGridTest.php new file mode 100644 index 0000000000000..22b559df5c656 --- /dev/null +++ b/tests/Unit/Libraries/Cms/Html/HtmlGridTest.php @@ -0,0 +1,142 @@ + + * @license GNU General Public License version 2 or later; see LICENSE.txt + */ + +namespace Joomla\Tests\Unit\Libraries\Cms\Html; + +use Joomla\CMS\Application\CMSWebApplicationInterface; +use Joomla\CMS\Document\Document; +use Joomla\CMS\Factory; +use Joomla\CMS\HTML\Helpers\Grid; +use Joomla\CMS\HTML\Registry; +use Joomla\CMS\Language\Language; +use Joomla\CMS\WebAsset\WebAssetManager; +use Joomla\DI\Container; +use Joomla\Tests\Unit\UnitTestCase; +use PHPUnit\Framework\MockObject\MockObject; + +/** + * Test class for Grid. + * + * @package Joomla.UnitTest + * @subpackage Html + * @since __DEPLOY_VERSION__ + */ +class HtmlGridTest extends UnitTestCase +{ + /** + * The web asset manager of the document + * + * @var WebAssetManager&MockObject + * @since __DEPLOY_VERSION__ + */ + private $wa; + + /** + * The factory state replaced by the test + * + * @var array + * @since __DEPLOY_VERSION__ + */ + private $factoryState; + + /** + * Sets up an application whose document has a web asset manager + * + * @return void + * + * @since __DEPLOY_VERSION__ + */ + protected function setUp(): void + { + $this->wa = $this->createMock(WebAssetManager::class); + + $document = $this->createStub(Document::class); + $document->method('getWebAssetManager')->willReturn($this->wa); + + $app = $this->createStub(CMSWebApplicationInterface::class); + $app->method('getDocument')->willReturn($document); + + $language = $this->createStub(Language::class); + $language->method('_')->willReturnArgument(0); + + $container = new Container(); + $container->set(Registry::class, new Registry()); + + $this->factoryState = [Factory::$application, Factory::$language, Factory::$container]; + Factory::$application = $app; + Factory::$language = $language; + Factory::$container = $container; + } + + /** + * Restores the factory state + * + * @return void + * + * @since __DEPLOY_VERSION__ + */ + protected function tearDown(): void + { + [Factory::$application, Factory::$language, Factory::$container] = $this->factoryState; + + parent::tearDown(); + } + + /** + * Tests that the default check all toggle uses the list-view script instead of an inline handler + * + * @return void + * + * @since __DEPLOY_VERSION__ + */ + public function testCheckallHasNoInlineHandler() + { + $this->wa->expects($this->once())->method('__call')->with('useScript', ['list-view'])->willReturnSelf(); + + $this->assertSame( + '', + Grid::checkall() + ); + } + + /** + * Tests that a custom check all action is kept + * + * @return void + * + * @since __DEPLOY_VERSION__ + */ + public function testCheckallKeepsCustomAction() + { + $this->wa->expects($this->once())->method('__call')->with('useScript', ['core'])->willReturnSelf(); + + $this->assertSame( + '', + Grid::checkall('toggle', 'myCheckAll(this)') + ); + } + + /** + * Tests that the state filter uses the list-view script instead of an inline handler + * + * @return void + * + * @since __DEPLOY_VERSION__ + */ + public function testStateHasNoInlineHandler() + { + $this->wa->expects($this->once())->method('__call')->with('useScript', ['list-view'])->willReturnSelf(); + + $html = Grid::state(); + + $this->assertStringContainsString('', $html); + $this->assertStringContainsString('', $html); + $this->assertStringNotContainsString('onchange', $html); + } + + /** + * Tests that an administrator pagination link uses the list-view script instead of an inline handler + * + * @return void + * + * @since __DEPLOY_VERSION__ + */ + public function testAdministratorPageLinkHasNoInlineHandler() + { + $wa = $this->createMock(WebAssetManager::class); + $wa->expects($this->once())->method('__call')->with('useScript', ['list-view'])->willReturnSelf(); + + Factory::$application = $this->getApplication('administrator', $wa); + + $html = $this->renderPageLink(new PaginationObject('3', 'list', 40, 'index.php?limitstart=40')); + + $this->assertStringContainsString('href="#" data-limitstart-field="listlimitstart" data-limitstart="40" class="page-link js-grid-pagination-link"', $html); + $this->assertStringNotContainsString('onclick', $html); + } + + /** + * Tests that a site pagination link stays a plain link + * + * @return void + * + * @since __DEPLOY_VERSION__ + */ + public function testSitePageLinkIsAPlainLink() + { + $wa = $this->createMock(WebAssetManager::class); + $wa->expects($this->never())->method('__call'); + + Factory::$application = $this->getApplication('site', $wa); + + $html = $this->renderPageLink(new PaginationObject('3', '', 40, 'index.php?limitstart=40')); + + $this->assertStringContainsString('href="index.php?limitstart=40" class="page-link"', $html); + $this->assertStringNotContainsString('onclick', $html); + } + + /** + * Returns an application of the client, whose document has the web asset manager + * + * @param string $client The application client + * @param WebAssetManager $wa The web asset manager + * + * @return CMSApplication + * + * @since __DEPLOY_VERSION__ + */ + private function getApplication(string $client, WebAssetManager $wa): CMSApplication + { + $document = $this->createStub(Document::class); + $document->method('getWebAssetManager')->willReturn($wa); + + $app = $this->createStub(CMSApplication::class); + $app->method('getName')->willReturn($client); + $app->method('isClient')->willReturnCallback(fn ($identifier) => $identifier === $client); + $app->method('getDocument')->willReturn($document); + + $router = $this->createStub(Router::class); + $router->method('getVars')->willReturn([]); + Factory::$container->set(ucfirst($client) . 'Router', $router); + + return $app; + } + + /** + * Renders the pagination link layout for an active item + * + * @param PaginationObject $item The pagination item + * + * @return string + * + * @since __DEPLOY_VERSION__ + */ + private function renderPageLink(PaginationObject $item): string + { + return LayoutHelper::render('joomla.pagination.link', ['data' => $item, 'active' => true], JPATH_ROOT . '/layouts', ['component' => 'none']); + } +}